Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor: custom icon sizing #1157

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

RubenSmn
Copy link
Member

Description

Will improve the resizing on mobile, first it was a fixed 28px now it is 75% of the initial(desktop) value.
All can be overridden with the sx prop, a good example is in the ImpactSection.

Reason for the change is that there is no need to set a different height and width since the svg will be rendered as a square anyway.

  • add size prop to CustomIcon component
  • update all components to use new size prop
  • updated some icon sizes to improve ux

Type of change

  • Enhancement

Checklist:

  • I have performed a self-review of my own code
  • My changes generate no new warnings

@RubenSmn
Copy link
Member Author

RubenSmn commented Dec 5, 2022

@dadiorchen we should consider this. It will greatly improve the UI of our icons, especially on mobile.

@dadiorchen
Copy link
Collaborator

@RubenSmn okay, I will download this and run it to check locally, can you fix the conflict for me?

@RubenSmn
Copy link
Member Author

RubenSmn commented Dec 6, 2022

@dadiorchen done!

@dadiorchen dadiorchen changed the base branch from beta to main June 28, 2023 09:52
@dadiorchen
Copy link
Collaborator

@RubenSmn do you think this is still valid or we can close it?

@RubenSmn
Copy link
Member Author

@dadiorchen I think it still could be valuable since it standardizes the size of a icon. For example on the top page: https://beta-map.treetracker.org/top the tree icons on the leader board have a incorrect size.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants