Repository navigation
Document Material 3 fallback for Divider.color and VerticalDivider.color - #193136
gbolahan507 wants to merge 1 commit into
Conversation
The [Divider.color] and [VerticalDivider.color] docstrings previously stated that when the color and DividerThemeData.color are both null, [ThemeData.dividerColor] is used. In Material 3 (the default since 3.16) the effective fallback is actually [ColorScheme.outlineVariant], as already documented on [Divider.createBorderSide]. Align the public field docs with the actual behavior. Fixes: flutter#190534
|
This pull request contains changes to Material or Cupertino, which are currently frozen in this repository. Changes should be made in Please refer to #188444 for instructions. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the documentation for both Divider and VerticalDivider to specify that when ThemeData.useMaterial3 is true, the default color falls back to ColorScheme.outlineVariant instead of ThemeData.dividerColor. The feedback suggests rephrasing these documentation comments to avoid a comma splice and nested conditional structure, which improves overall readability.
| /// also null, then if [ThemeData.useMaterial3] is true it defaults to | ||
| /// [ColorScheme.outlineVariant], otherwise [ThemeData.dividerColor] is used. |
There was a problem hiding this comment.
The phrasing here contains a comma splice and is slightly hard to read due to the nested conditional structure ('then if...'). Rephrasing it to use a semicolon and placing the condition after the default value improves readability and grammatical correctness.
/// also null, then it defaults to [ColorScheme.outlineVariant] if\n /// [ThemeData.useMaterial3] is true; otherwise, [ThemeData.dividerColor] is used.References
- Optimize for readability: Code is read more often than it is written. (link)
| /// also null, then if [ThemeData.useMaterial3] is true it defaults to | ||
| /// [ColorScheme.outlineVariant], otherwise [ThemeData.dividerColor] is used. |
There was a problem hiding this comment.
The phrasing here contains a comma splice and is slightly hard to read due to the nested conditional structure ('then if...'). Rephrasing it to use a semicolon and placing the condition after the default value improves readability and grammatical correctness.
/// also null, then it defaults to [ColorScheme.outlineVariant] if\n /// [ThemeData.useMaterial3] is true; otherwise, [ThemeData.dividerColor] is used.References
- Optimize for readability: Code is read more often than it is written. (link)
github-actions
Bot
added
framework
|
Closing per the code-freeze CI failure — Material changes now live in flutter/packages material_ui (see #188444). Ported to flutter/packages#12967. |
…icalDivider.color (#12967) Port of flutter/flutter#193136 — the original PR could not land on flutter/flutter due to the Material/Cupertino code freeze on `packages/flutter/lib/src/material` (see flutter/flutter#188444). Re-opening the doc fix here in `material_ui`. The `Divider.color` and `VerticalDivider.color` docstrings currently state that when both the local `color` and `DividerThemeData.color` are null, `ThemeData.dividerColor` is used. In Material 3 (the default since Flutter 3.16) the effective fallback is actually `ColorScheme.outlineVariant`, as `Divider.createBorderSide` already documents. This PR aligns the public field docstrings with the actual runtime behavior. Fixes: flutter/flutter#190534 Notes for reviewers: - The `Divider.color` docstring is a `{@template material_ui.Divider.color}` reused via `{@macro}` elsewhere, so the fix propagates. - `VerticalDivider.color` carries a duplicate (non-macro) docstring, which has been updated to match. - A prior attempt on flutter/flutter (#190535) was approved by @elliette before being auto-closed for inactivity on a whitespace nit; this port takes that direction into the correct repo per the decoupling policy.
The
Divider.colorandVerticalDivider.colordocstrings currently state that when both the localcolorandDividerThemeData.colorare null,ThemeData.dividerColoris used. In Material 3 (the default since Flutter 3.16) the effective fallback is actuallyColorScheme.outlineVariant, asDivider.createBorderSidealready documents. This PR aligns the public field docstrings with the actual runtime behavior.Fixes: #190534
Notes for reviewers:
Divider.colordocstring is a{@template flutter.material.Divider.color}that is also{@macro}-referenced byPopupMenuDivider, so the fix propagates to that widget's docs as well.VerticalDivider.colorhas a duplicate (non-macro) docstring, which has been updated to match.Pre-launch Checklist
///).