-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Elements: Add UI for button elements #41659
Conversation
Size Change: -2 B (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
a5bdad6
to
7f74df5
Compare
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.
Nice work on getting this PR up 👏
I gave it a spin with TT2 and I'd say it's 80% of the way. I noticed:
- font size has no effect
- colors have no effect
Screen.Capture.on.2022-06-21.at.09-04-21.mov
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.
Reviewed this again but this time with Empty Theme (incidentally I notcied the reason I used TT2 on my first review was because that was shown in the demo screenrecording on the PR).
Unfortunately I still get the same results.
Screen.Capture.on.2022-06-22.at.10-26-44.mov
I believe the issue is that CSS styles are overloading the global style rules:
I can't seem to dismiss my previous reviews. However, having spoken with @draganescu he's explained that #41822 actually fixes a number of the CSS specificity Issues I identified with this PR. Therefore we should wait for that PR to be merged and this PR to be rebased before attempting further reviews. |
bcbaac4
to
1e78905
Compare
Thanks for picking this up and running with it @draganescu. LGTM |
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.
I tested this again and it now seems to work a lot better. I can adjust both color and typography for buttons and it propagates.
Screen.Capture.on.2022-06-22.at.15-51-51.mov
This only works with Empty Theme for now because other Themes are still adding styles for button elements in CSS rather than in the new improved .json
way.
Nice work
What?
#41246 introduced support for the
button
element intheme.json
. This adds a UI to make it possible to edit button elements in Global Styles.Why?
To allow users to edit their button styles.
How?
Duplicating existing global styles controls.
Testing Instructions
Screenshots or screencast
Typography
button-gs.mp4