Create shared ruler and standardize across views #2240
Reference in New Issue
Block a user
Delete Branch "rulers-dividers"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
When building out some of the design for the rules, one of the things that came up was how the divider between each rule should look. There were quite a few ways this was done before -
<hr>, a<div>withh-px, some places had arulerpartial, even an instance ofborder-b last:border-b-0.This PR introduces a new pattern to standarize all of this a bit with the usage of a
shared/rulerpartial. This ruler uses theborder-dividerutility (an alias ofborder-tertiary, if this is a dumb alias happy to change it), with a defualt ofmx-4to handle the majority use case of separating items in a list (tags, rules, categories, merchants, etc).This new partial is very small, but I think is required for use with the
spacer_templatefor collection rendering (I also moved a few things over to this pattern to match some of the others). The normal<hr ..>line is easy enough to put everywhere but automatic spacers is nice, so I thought I'd extend it to everything.Right now I allow customization through a
classeslocal, for places that require a full width spacer or y padding. In general, there are very few actual design differences in this PR. There were a few lists where the spacer did not appear under the icon where it does now, but things are consistent now. Here's an example from merchants:If there's a different direction for this that is better/more idiomatic, I'm happy to adjust!
Agree on consolidating all of these into a shared partial!
I'm not 100% sure on the design end whether the divider would always have the same type of padding, so will defer to @justinfar on that one.
I agree that they probably won't all be the same, which is why I decided to have the
classesoption. I do use this in a few places to maintain the current styling (e.g. a full-width line on the menu popupIf we want we could avoid any kind of default and pass in the padding/other each time, but I opted to default to the most common at this point. Definitely happy to tweak this though.
@ahatzz11 yeah I think what you've got with the
classesis all good. I was mainly referring to the dividers in this screenshot:@justinfar are these supposed to be the same across the app?
@zachgoll @ahatzz11
They’re not meant to be the same across the app, we use different divider styles depending on the context, so it's fairly flexible in terms of usage. I'll need to set some time and define this in the design system.
There's three main patterns for dividers within list items:
In your screenshot @ahatzz11, the padded divider works well for the Merchants table. But if that were a regular list with multiple related items (as is usually the case with that component), a leading icon with content divider might make more sense to maintain grouping.
Okay that is good to know! Having something that shows what should use which type of divider would be awesome - and maybe in the future we implement some kind of
variantfor the ruler/divider so that's a bit more standardized.The merchant one (in my screenshot) is one of two things where I changed the design - so good to know that one is fine! The other one I changed was the accounts on the dashboard balance sheet. Here is a before and after. I think the right padding on the before in this is a bit strange, but I can see why it would make sense to have the divider to be past the icon on the left in this instance:
Here's what it would look like with the the leading icon with divider:
@ahatzz11 I'll be honest, my design sense isn't the greatest, so will need @justinfar to do final approvals on this.
@justinfar what's the correct padding for those account groups?
Let's use the one the leading icon with content divider for the account groups, which you showed in your last image @ahatzz11
Gap is 12px and each of these list items have a padding of
16pxfor L,R and12pxfor T,BThe nested list items have a left padding of
48px@justinfar sounds great! I think this is all updated and complete now. Here are some additional screenshots showing off dividers throughout the app from this PR:
Looks great! Thanks for consolidating all of these.