Skip to content

Create the IconRegistry#11982

Merged
andreslucena merged 44 commits intodevelopfrom
fix/11965
Nov 21, 2023
Merged

Create the IconRegistry#11982
andreslucena merged 44 commits intodevelopfrom
fix/11965

Conversation

@alecslupu
Copy link
Copy Markdown
Contributor

@alecslupu alecslupu commented Nov 10, 2023

🎩 What? Why?

Please describe your pull request.

📌 Related Issues

Link your PR to an issue

Testing

Describe the best way to test or validate your PR.

📷 Screenshots

image

♥️ Thank you!

Copy link
Copy Markdown
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This pull request does not contain a valid label. Please add one of the following labels: ['type: feature', 'type: change', 'type: fix', 'type: removal', 'target: developer-experience', 'type: internal']

@alecslupu
Copy link
Copy Markdown
Contributor Author

@andreslucena , @Crashillo have a look on this PR, if you agree with the approach, i will go and extract all the svg icons in their own modules ( some modules may have custom icons like the ones from;

"Decidim::Proposals::CollaborativeDraft" => { icon: "draft-line", description: "Collaborative draft", category: "activity" },
"Decidim::Proposals::Proposal" => { icon: "chat-new-line", description: "Proposal", category: "activity" },
"Decidim::Amendment" => { icon: "git-branch-line", description: "Amendment", category: "activity" },
"Decidim::ParticipatoryProcess" => { icon: "treasure-map-line", description: "Participatory Process", category: "activity" },
"Decidim::Budgets::Budget" => { icon: "coin-line", description: "Budget", category: "activity" },
"Decidim::Budgets::Project" => { icon: "coin-line", description: "Project (Budgets)", category: "activity" },

@alecslupu alecslupu added type: fix PRs that implement a fix for a bug project: redesign Barcelona City Council contract labels Nov 10, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 10, 2023
@andreslucena
Copy link
Copy Markdown
Member

@andreslucena , @Crashillo have a look on this PR, if you agree with the approach, i will go and extract all the svg icons in their own modules ( some modules may have custom icons like the ones from;

So, this would also fix #11137, right?

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is just what I had in mind 😄

@Crashillo
Copy link
Copy Markdown
Contributor

I will mention @entantoencuanto to discuss about the approach, since the Ruby/Rails is not my strong point 😅

@alecslupu
Copy link
Copy Markdown
Contributor Author

@andreslucena , @Crashillo have a look on this PR, if you agree with the approach, i will go and extract all the svg icons in their own modules ( some modules may have custom icons like the ones from;

So, this would also fix #11137, right?

I think it would as, it would enable us to call it from whatever place we would like.

@entantoencuanto
Copy link
Copy Markdown
Contributor

Hi, I think that this approach is a great improvement! My only doubt is if you are considering to provide a method to be used by the design application to get all the icons and group them by category. Currently this is done accessing directly to the icons hash in the helper and grouping. But the IconRegistry class does not provide a public method returning this hash or a structure grouped by category

@alecslupu
Copy link
Copy Markdown
Contributor Author

But the IconRegistry class does not provide a public method

This will be added ... I just wanted to validate the approach on the matter.

github-actions[bot]
github-actions bot previously approved these changes Nov 10, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 11, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 11, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 11, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 15, 2023
@andreslucena
Copy link
Copy Markdown
Member

I've updated the issues that'll be closed when we merge this one 😄

Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All in all I'm really happy with how this looks like. Now we have a documentation system with all the Decidim icons 🚀

For now I'll ignore two things:

  1. The categories: we should probably rethink them to something that makes more sense. I'll leave that out of the scope of the current PR, only asking for one small change, of merging the "Action" and "Actions" categories. Most likely is that I'll review the categories with @decidim/product in the future to propose a reorganization
  2. The descriptions: as of now it's missing in lots of icons, it's out of the scope of the current PR so we don't drag this merge.

Apart from that I think I found a couple minor things, can you check them out please?

alecslupu and others added 2 commits November 20, 2023 10:39
Co-authored-by: Andrés Pereira de Lucena <andreslucena@users.noreply.github.com>
github-actions[bot]
github-actions bot previously approved these changes Nov 20, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 20, 2023
github-actions[bot]
github-actions bot previously approved these changes Nov 20, 2023
Copy link
Copy Markdown
Member

@andreslucena andreslucena left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏽 great job

@andreslucena andreslucena merged commit 354f9bc into develop Nov 21, 2023
@andreslucena andreslucena deleted the fix/11965 branch November 21, 2023 09:14
@andreslucena andreslucena added target: developer-experience and removed type: fix PRs that implement a fix for a bug labels Nov 21, 2023
@alecslupu alecslupu mentioned this pull request Nov 21, 2023
entantoencuanto added a commit that referenced this pull request Nov 27, 2023
* develop: (51 commits)
  Fix a11y violations and title layout on announcements (#12067)
  Fix randomness on nickname and emails for seeded users (#12058)
  Fix render partial deprecation warning on proposals (#12035)
  Redesign: proposal counter total number avaliable (#12014)
  Fix disabling access to space private users when the space is public (#12031)
  Redesign: admin screen overflow on smaller screens (#11956)
  Make the breadcrumb consistent in the components import pages (#12024)
  Fix develop branch (#12051)
  Create the IconRegistry (#11982)
  Fix process groups component content blocks (#11872)
  Remove the asemblies settings page (#11873)
  Fix n+1 query on assemblies permissions (#12040)
  Remove unused scope filter from search controller and command (#12038)
  Fix issues in "Verify your identity" (#12030)
  Fix flash of dropdowns in buttons from the title bar and "Manage" button (#12011)
  Add CI workflow configuration for decidim-design (#12022)
  Extract methods from participatory processes seeds (#12032)
  Show message when there are no debates (#12037)
  Show error message when there are no budgets nor projects in budgets (#12034)
  Fix faker requirement in production environment 2 (#12033)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Make a system for documenting all the icons Refactor icon resource logic

4 participants