Skip to content

refactor: custom icon sizing#1157

Open
RubenSmn wants to merge 3 commits intoGreenstand:mainfrom
RubenSmn:refactor/custom-icon-sizing
Open

refactor: custom icon sizing#1157
RubenSmn wants to merge 3 commits intoGreenstand:mainfrom
RubenSmn:refactor/custom-icon-sizing

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 RubenSmn requested a review from dadiorchen December 5, 2022 10:20
@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 RubenSmn force-pushed the refactor/custom-icon-sizing branch from b4f57b4 to a51fc60 Compare December 6, 2022 07:36
@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