-
Notifications
You must be signed in to change notification settings - Fork 179
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
feat(style): new style proposal #1948
base: develop
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM - but will wait for 1 more core contributor to provide their opinions before this can be merged
@SpecialAro / @Alphrag - wdyt?
72fc615
to
b198427
Compare
…m-app into feature/new-style
LGTM as well, but I'll suggest to add a setting option to change the padding (so users can remove the padding or adjust it according to their needs) |
Alright, makes sense. I'll start to do this change. Do you prefer:
The first one is what I would choose. It doesn't give the user the option to set another value, but it will give the option to show or hide. It is most simple and it would be easy to maintain or to other contributors to understand. If we really want to give other options to the user, I would prefer the second option as it will give us the control of the values, it would be easier to maintain and prevent issues that could appear by letting the user select any value for the paddings. |
This would be the first option: Screencast_20241028_110918.webm |
…dding size to 6px
@DenysMb - please rebase and fix conflicts. btw - is this PR still needed/valid? |
It is more a proposal than something that is needed. You can close this PR. |
Pre-flight Checklist
Please ensure you've completed all of the following.
Description of Change
Changed the tab style and added paddings and border-radius in the service view (this one has this separated PR in if you like the idea of having these paddings but not changing the tab style #1947).
Motivation and Context
New applications like Arc Browser (screenshot 1 and screenshot 2) have been using this style of buttons in the sidebar with paddings, gaps and border-radius. I think that this would give Ferdium a new modern yet minimalist style.
Screenshots
Checklist
pnpm prepare-code
)pnpm test
passesRelease Notes
Changed the tab style and added paddings and border-radius in the service view.