feat(List): add small prop, boxedDefaultPadding token and update divider alignment - #1677
feat(List): add small prop, boxedDefaultPadding token and update divider alignment#1677MurilloLeoni wants to merge 14 commits into
Conversation
|
Label 'safe-to-deploy' added — dispatching fork preview workflow. Awaiting environment approval to expose deploy secrets. |
Marcosld
added
safe-to-deploy
|
I would have split into several PRs to ease focusing on the thing you're changing, and having a 1-1 relation with the subtasks |
aweell
left a comment
aweell
left a comment
There was a problem hiding this comment.
Should screenshot examples for small list variant be included?
🚀 Fork preview deploy triggeredThe Important A reviewer must approve the workflow run before it can access deploy secrets. |
Vercel Preview
|
|
|
||
| const RowListDividerContext = React.createContext(false); | ||
|
|
||
| type ListContextType = {small: boolean}; |
There was a problem hiding this comment.
not sure if we wanna introduce such a generic provider, or narrow it down to a specific small provider. have you considered it?
type ListCompactContextType = {small: boolean};
const ListCompactContext = React.createContext<ListCompactContextType>({small: false});
| ))} | ||
| </div> | ||
| <ListContext.Provider value={{small}}> | ||
| <RowListDividerContext.Provider value> |
There was a problem hiding this comment.
this is weird right? value should be at least true or false I think 🤔.
does @Telefonica/mistica-design agree with this behavior? showing/hiding the dividier
There was a problem hiding this comment.
hi! setting it explicitly as value={true} actually breaks the react/jsx-boolean-value lint rule (screenshot attached), so the shorthand was the way to go here.
There was a problem hiding this comment.
yes but why do you wanna it in the first place? isn't the hideDivider enough? why the context, which is always true?
There was a problem hiding this comment.
Make sense. I’m thinking of refactoring Content so it no longer reads RowListDividerContext directly and instead only receives a showDivider?: boolean prop.
RowContent would become the only place that consumes the context and computes the final value before passing it down. The divider condition in Content would then become much simpler:
{showDivider && (
<div className={styles.rowDivider}>
...
</div>
)}This would keep the context usage contained in a single place and make the divider logic more explicit.
Wdyt?
|
@MurilloLeoni can you link to the spec of the list component with the updates? |
| import type {DataAttributes, TrackingEvent, IconProps} from './utils/types'; | ||
| import type {ExclusifyUnion} from './utils/utility-types'; | ||
|
|
||
| const RowListDividerContext = React.createContext(false); |
There was a problem hiding this comment.
why have you introduced this provider? is the divider specification changing too? and actually, I see it always true (useless then?) + a weird branching
{hasDivider && !hideDivider && (
on line 289. I don't know: can you explain it better?
There was a problem hiding this comment.
the RowListDividerContext defaults to false, and only RowList sets it to true.
I introduced it so Content can know when it is being used inside a RowList. This replaces the previous approach where Divider elements were added manually between rows in RowList.
Components such as BoxedRowList use Content without this provider, so hasDivider stays false and no divider is rendered there.
The hasDivider && !hideDivider condition is needed for the dualAction layout (for example, a row with both onPress and a switch).
In that case, Content is rendered inside the BaseTouchable on the left side only. If the divider were rendered there, it would cover only the touchable area, not the full row including the control on the right. For that reason, hideDivider prevents the internal divider, and a separate divider is rendered below the whole row instead.
hii! Just to clarify, would you prefer that we split this current PR into several smaller ones, or is this more of a guideline to follow for the remaining subtasks and future PRs? |
Sure! I’ll add the updated Lists specification link to the PR description and send here. |
no worries, just for the future! |
There was a problem hiding this comment.
why boxedDefaultPadding is imported but never used?
There was a problem hiding this comment.
i checked the current src/list.tsx and I can’t find any boxedDefaultPadding import there. I think the comment may be referring to an older diff or another file.
5 participants
This PR updates
RowListandBoxedRowList.Changes
smallprop for compact row rendering, reducing text scale across title, subtitle, description and detail.Create Small variant to RowList and BoxedRowList mistica-design#2676
boxedDefaultPaddingtoken to control horizontal padding, replacing previously hardcoded values.Adjust horizontal padding in Row and Accordion components mistica-design#2679
font-weightvia the updated design token from mistica-design.Change the value of the rowTitle token for Vivo Evolution skin mistica-design#2684
Change divider width in RowList mistica-design#2678
Related
Spec
https://www.figma.com/design/Be8QB9onmHunKCCAkIBAVr/%F0%9F%94%B8-Lists-Specs?node-id=0-1