refactor: modernize Divider to latest MD3 spec - #5089
Conversation
JKobrynski
left a comment
There was a problem hiding this comment.
One minor comment, we've also got conflicts
|
|
||
| ## Components | ||
|
|
||
| ### Divider |
There was a problem hiding this comment.
This conflicts with main now - #5078 landed a ## General changes section at the same insertion point. Both sides only add text, so keeping both resolves it.
There was a problem hiding this comment.
Rebased on main and resolved the conflicts:
- docs/6.x/docs/guides/migration.md: kept both sections: this PR's new ### Divider entry plus refactor: rework Surface and elevation shadows #5078's ## General changes section, as suggested.
- docs/6.x/docs/components/Divider.mdx: removed, following main's deletion. main migrated to auto-generated component docs (from JSDoc in source), and the accessibility note this PR added here already lives in Divider.tsx's JSDoc, so nothing was lost.
- docs/src/data/componentDocs6x.json: removed, also following main's deletion (a generated data file for the old docs system, no longer used).
# Conflicts: # docs/6.x/docs/components/Divider.mdx # docs/6.x/docs/guides/migration.md # docs/src/data/componentDocs6x.json
|
|
||
| return ( | ||
| <View | ||
| aria-hidden |
There was a problem hiding this comment.
Tests that look up a divider will stop finding it - aria-hidden keeps it out of getByTestId unless the query passes { includeHiddenElements: true }. Worth a line in the migration guide?
There was a problem hiding this comment.
Good catch, I've added a note in the migration guide. Also fixed latest conflicts.
JKobrynski
left a comment
There was a problem hiding this comment.
LGTM in general, one minor comment + we've got conflicts
# Conflicts: # src/components/Divider.tsx
Motivation
Dividerwas drawing aStyleSheet.hairlineWidthline, which is thinner than 1dp on most screens, and the full 1dp line the Material Design 3 spec asks for was only reachable throughbold.leftInsetsetmarginLeft, so in RTL the inset stayed on the left instead of moving to the leading edge. MD3 also lists a vertical divider, which we had no way to render.This is primitive level work for the v6 effort. The consumers (List, Dialog, Drawer, DataTable) pick up the corrected divider in their own tasks.
Changes
StyleSheet.hairlineWidth→1dp, applied asheightwhen horizontal andwidthwhen verticalleftInset(marginLeft: 16) →startInset(marginStart: 16), so it follows the writing directionorientation="horizontal" | "vertical". A vertical divider is 1dp wide and usesalignSelf: 'stretch'to take the height of its parentstartInsetinsets the top edge andhorizontalInsetinsets the top and bottom edgesaria-hiddenby default, so screen readers skip it and it stays out of the focus order. React Native maps that toaccessibilityElementsHiddenon iOS andimportantForAccessibility="no-hide-descendants"on Android. It is spread before...rest, so<Divider accessible aria-hidden={false} role="separator" />opts back inBreaking change
The
boldandleftInsetprops have been removed.boldexisted only to reach 1dp, which is now the default. For the old hairline line, passstyle={{ height: StyleSheet.hairlineWidth }}.The full entry is in
docs/6.x/docs/guides/migration.md.Callsites updated
src/components/Searchbar.tsxandsrc/components/Drawer/DrawerSection.tsxdroppedbold, so they render the same as beforeexample/src/Examples/DividerExample.tsxnow shows full width, start inset, both side inset and vertical dividersexample/src/Examples/FABExample.tsxdroppedboldsrc/components/__tests__/Drawer/__snapshots__/DrawerSection.test.tsx.snapupdatedRelated issue
N/A
Test plan
yarn lintcleanyarn typecheckclean, plustsc --noEmitinexample/yarn test56 suites, 743 passed, including 11 newDividertests insrc/components/__tests__/Divider.test.tsxyarn docs buildclean,Divider.mdxandcomponentDocs6x.jsonregeneratedTo check it by hand, open the example app, go to the Divider screen and compare against
main. The line is visibly thicker, and the vertical section is new.Visual verification