-
Notifications
You must be signed in to change notification settings - Fork 3.4k
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
Reorder color setting in Header #2231
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.
Left one feedback. The rest is looking good 👍.
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.
One thing I think we can further clean up. Otherwise looks good and I'm aligned with the approach to remove the headers.
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.
Good job 🚀
Reorder color setting in Header (Shopify#2231)
* reorder color setting * add extra header * move below separator line * testing alternative * remove header content * changing content to info * testing locale change * changes * revert * remove unused content
PR Summary:
Reorders the "Color scheme" setting lower in the settings list to improve settings hierarchy.
Why are these changes introduced?
Fixes #2232
What approach did you take?
Reordered the settings
Other considerations
We might improve those patterns hierarchy later but wanted to improve this order sooner for the Header section.
Decision log
Visual impact on existing themes
Testing steps/scenarios
Color scheme
setting should be below theShow separator line
setting.Demo links
Checklist