Upgrade bitstyles to 5.0.1 #125
Conversation
|
Waiting for bitstyles |
angelikatyborska
changed the title
|
Note to self: fix heading with list, the list should not look like a list Edit: done |
lib/bitstyles_phoenix/bitstyles.ex
Outdated
| case class do | ||
| "u-list-none" -> "a-list-reset" | ||
| "a-badge--text" -> "a-badge--gray" | ||
| "u-gap-l" -> "u-gap-m" |
There was a problem hiding this comment.
🤔 this will change the behaviour quite a lot, right? We have numerous places in the code in dima e.g. that use u-gap-l already. They will all be using u-gap-m afterwards if they pass through this. But there is also only one bitstyles helper using u-gap-m. Mabye we can instead simply list this as a change for ui_dl_item, since one can always pass in an overriding u-gap-l into the classes and not add this migration here.
After all size can also be maintained by setting the gap size in sass.
There was a problem hiding this comment.
I'm not sure I understand...
We have numerous places in the code in dima e.g. that use u-gap-l already. They will all be using u-gap-m afterwards if they pass through this.
Isn't this exactly what we want? The class u-gap-m is 4.3.0 has the same styles as u-gap-l in 5.0.0. The name of the class changed, but the exactly rem value used for the gap remained the same.
There was a problem hiding this comment.
But then are the values for u-gap-m and u-gap-l then same value by default now?
What I am saying is also that we do not make an assumption on the "migratedness" of foreign code that uses the helpers. With this setting you basically can't use u-gap-l anymore with bistyles 4.3.0 even if you want to have that value if you have classes going through this helper.
There was a problem hiding this comment.
Also keep in mind the values are always just values set by the sass variables, so one can always go back to the old values if they want to not migrate things.
There was a problem hiding this comment.
But then are the values for u-gap-m and u-gap-l then same value by default now?
The class u-gap-m does not exist in the default configuration of bitstyles in 5.0.0, see:
https://github.com/bitcrowd/bitstyles/blob/v5.0.1/scss/bitstyles/utilities/gap/_settings.scss#L4-L7
What I am saying is also that we do not make an assumption on the "migratedness" of foreign code that uses the helpers. With this setting you basically can't use u-gap-l anymore with bistyles 4.3.0 even if you want to have that value if you have classes going through this helper.
I think I understand. It's fine to define class mappings in this module if the new class only exists in the new version, and the old class only exists in the old version. But here we have a new class that exists in both new and old version, but with a different meaning in the old version (u-gap-l from 4.3.0 would be equivalent to u-gap-xl in 5.0.0)
There was a problem hiding this comment.
lib/bitstyles_phoenix/bitstyles.ex
Outdated
|
|
||
| class = | ||
| Enum.reduce(sizes_renaming, class, fn {new_size, old_size}, acc -> | ||
| String.replace(acc, "-#{new_size}", "-#{old_size}") |
There was a problem hiding this comment.
Shouldn't we at least make sure here, that the class starts with u- and is a bitstyles class?
There was a problem hiding this comment.
| |> Keyword.put_new(:variant, "menu") | ||
|
|
||
| extra = | ||
| if Bitstyles.version() >= "5.0.0" do |
There was a problem hiding this comment.
We really have to start refactoring version comparisons to not use strings at some point 🙈
But good for now.
There was a problem hiding this comment.
Yes 😇 I'll try to squeeze in that work in my schedule after 5.0.0 and 6.0.0 upgrades are merged
Resolves #122
Note that the sidebar example in 4.3.0 will look weird because it contains a lot of example content that's written with attributes supported in 5.0.0 only.
Also note that the class
a-buttonis nowdisplay: flexinstead ofdisplay: inline-flexand now<a>s with that class look different than<button>s with that class due to different default widths of the two elements. This discrepancy would need to be fixed on a project level where bitstyles-phoenix is used, by something likea { width: fit-content }or so. Or just be using those elements inside appropriate flex/grid containers.List of changes:
u-gap-misu-gap-l, looks identicala-list-resetisu-list-nonea-badge--grayisa-badge--textu-fg-andu-bg-classes to be different. Darren helped me choose adequate replacements.