Fix/theme settings marketplace additions #461
Conversation
Updated node-version to accommodate the updates made to .node-version and eslint needed to allow Bend command to run.
| "step": 1, | ||
| "suffix": "px", | ||
| "default": 0 | ||
| }, |
There was a problem hiding this comment.
To keep things on the simple side do you think it could make sense to keep these condensed to one corner radius or do you think there is value in keeping these separated out? If we keep things separated out do you think it would sense to offer up bottom-corner radius options for the title if it will always sit on top of the form?
There was a problem hiding this comment.
I 100% Think there is value in this. I can see instances where folks might want to either style:
- title only
- form only
- title and form w/ various combos of border && background color.
There really are a lot of different options to account for and designers get pretty crazy. I def think it's best to give those options to the user. We could slim it down visually into a single number field, add a boolean for editing all corners individually and then displaying the rest. But, at that rate, we might as well see if the fields team can create a field for corner radius similar to border fields.
If we keep things separated out do you think it would sense to offer up bottom-corner radius options for the title if it will always sit on top of the form?
I actually added this to the title section already :)
There was a problem hiding this comment.
If we keep things separated out do you think it would sense to offer up bottom-corner radius options for the title if it will always sit on top of the form?
I misread this.
I do think it makes sense to keep the bottom margins because of the use cases in the reference images. I can foresee times when a designer might only style either the title section or the form. In those instances we would want them to be able to style them fully and independently. While it adds some extra options -- I think it ultimately gives the users much more freedom in design.
There was a problem hiding this comment.
Cool, that makes sense! I think as a follow-up issue/PR it could be worth checking around to see if there are other places corner-radius is used to make sure we have a similar set of options in each spot vs. the singular one 👍🏼
If we keep things separated out do you think it would sense to offer up bottom-corner radius options for the title if it will always sit on top of the form?
For this I more meant do you think a user would ever want the form title to have bottom border-radius options if it will always sit on top of a form (example of where I was thinking that could get funky below):
There was a problem hiding this comment.
@jasonnrosa I agree that it could get pretty funky. However, they would be responsible for handling that instance imo. And I think that is fair.
In my opinion the purpose of theme-settings is essentially to allow the user to effect change on global stylesheets without needing to know how to code. I def see that a user could potentially design themselves into a gross design. But I do feel like giving them open options makes the boilerplate a more flexible starting point.
So:
For this I more meant do you think a user would ever want the form title to have bottom border-radius options if it will always sit on top of a form (example of where I was thinking that could get funky below):
Not exactly -- but with the way this is handled on the growth theme we could not create a design like ref image 1. The closest you can get would be something like this
It doesnt work super well when both the title and form have Background colors but it does work well when one does and the other doesnt. ref image 2
Is that making sense?
There was a problem hiding this comment.
Yup - definitely understand now, thank you for the additional examples! This makes sense to me 👍🏼 I think my only follow-up ask is to just check through boilerplate for other instances of us using corner radius to see if it could get a similar update so we don't have more limited options in one spot but not another.
|
This is looking great - awesome work here 💪🏼 just added a few comments/questions above but everything else looked to work great during my tests! |
fixed incorrect json path for the inherited_value key. Should be working correctly now.
Update new useage of color fields to use the css color() macro. Some other small nit picky style guide tweaks.
hs-json-cleaner was run locally. No packages were added to the boilerplate.
Types of change
Description
Add additional theme settings to bring theme up to date with new marketplace requirements. #455
There was also an issue related to running the
bendcommand..node-versionfile.node-version(allows successful linting in git`).Relevant links
#455
Checklist
People to notify
@jasonnrosa