Closed andrewrubin closed 2 months ago
Latest commit: 9c1302ca4650236325c7ccbe03b1a0545d65e318
Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.
Click here to learn what changesets are, and how to add one.
Click here if you're a maintainer who wants to add a changeset to this PR
@andrewrubin thanks for the changes, this is looking great. I think that we should just always require the -
prefix. This way there is no ambiguity for contributors or confusion for component users.
These classes are generated with functions anyways, it's not like we are having to painstakingly update each one, so might as well make it a standard.
Good plan. So, to retrofit our grid-layout per your suggestion, we'll simply add the md-
prefix to the stylesheet? I'll do that now if so.
@marlonmarcello that's added — so we're now always adding the prefix (except on small, since, mobile-first 😉 ), and I've updated the <Column>
classnames to reflect that.
@andrewrubin I think we should add to small too mate. That way there is absolutely no questioning IF/WHEN/WHERE, they are always there, on all classes with breakpoints.
For sure, added the sm
prefix as well!
Description
This builds out the
buildBreakpointClassnames
helper a little bit, allowing for an optionalsm
property on the optionalprop
object. The motivation for this fix was to — by default — have the generated classnames encompass all breakpoints, rather than starting a medium (which was a bit too opinionated to the grid-layout system).Solution
You can now do something like
which will generate the following classnames:
.propName-yes
,.propName-md-no
, andpropName-lg-maybe
.Notes
sm
property, and adds or removed themd-
classname prefix based on the availability of said property. There is definitely a more robust solution floating out there in space — something that does a fancy loop and creates prefixes based on the availability of all breakpoint properties — but as I got into it, I decided it's smarter to just solve for our needs now and in the foreseeable future; rather than over-engineer things.Flex
stylesheet.Does this close any existing issues?
Closes #100 Referenced in #181 and #172 as well
Screenshots
Here's an example using the
<Flex>
component, and the newly availablesm
property. The difference between this and our current implementation is that there's now a difference between the small and medium breakpoints:https://github.com/wethegit/component-library/assets/30575213/741c0a42-f983-401f-b1a0-b81373f695a3