[docs] Add document content related to chip shape #140015

asci-00 · 2023-12-13T00:44:51Z

(#139572) I would like to request an update to the document to resolve confusion.
I actually modified the code by applying priority as in the comment in #139572 as shown below, but I thought that if 'OutlinedBorder' was used for 'shape', the default value of side for 'OutlinedBorder' was also the developer's intention.

OutlinedBorder _getShape(ThemeData theme, ChipThemeData chipTheme, ChipThemeData chipDefaults) {
  final BorderSide? resolvedSide =
      MaterialStateProperty.resolveAs<BorderSide?>(widget.side, materialStates)
          ?? MaterialStateProperty.resolveAs<BorderSide?>(chipTheme.side, materialStates);
  final BorderSide? resolvedShapeSide =
      MaterialStateProperty.resolveAs<BorderSide?>(widget.shape?.side, materialStates)
          ?? MaterialStateProperty.resolveAs<BorderSide?>(chipTheme.shape?.side, materialStates);
  final OutlinedBorder resolvedShape =
      MaterialStateProperty.resolveAs<OutlinedBorder?>(widget.shape, materialStates)
          ?? MaterialStateProperty.resolveAs<OutlinedBorder?>( chipTheme.shape, materialStates)
          ?? MaterialStateProperty.resolveAs<OutlinedBorder?>(chipDefaults.shape, materialStates)
          // TODO(tahatesser): Remove this fallback when Material 2 is deprecated.
          ?? const StadiumBorder();
  // If the side is provided, shape uses the provided side.

  if (resolvedSide != null) {
    return resolvedShape.copyWith(side: resolvedSide);
  }
  if (resolvedShapeSide != null) {
    return resolvedShape.copyWith(side: resolvedShapeSide);
  }
  // If the side is not provided
  // then the shape's side is used. Otherwise, the default side is used.
  return resolvedShape.copyWith(side: chipDefaults.side);
}

(in chip.dart)

However, (#133856) PR seems to be intended to ignore this and use the aspect of chipDefault.side even if the developer specifies shape.side as [Border.none] without explicitly applying side .
This is probably because the default value of OutlinedBorder is [Border.none].
I think this is an area that can cause enough confusion, but there are a lot of things that need to be modified to reduce confusion through code changes, and the impact on existing code is likely to be significant.
I also confirmed that this is not accurately explained in the API Docs.
So rather than modifying the code, I decided to add additional explanation to the shape comment.
If the results are different from what you intended or the updated content is strange, please review. 🙏

Pre-launch Checklist

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I signed the CLA.
I listed at least one issue that this PR fixes in the description above.
I updated/added relevant documentation (doc comments with ///).
I added new tests to check the change I am making, or this PR is test-exempt.
All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

google-cla · 2023-12-13T00:44:55Z

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

flutter-dashboard · 2024-01-04T22:39:58Z

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

HansMuller

LGTM

Piinks

Also LGTM! Thanks for contributing

cbracken

…utter#140015)

Roll Flutter from c6ddb73 to 126302d (32 revisions) flutter/flutter@c6ddb73...126302d 2024-01-09 [email protected] Roll Flutter Engine from 820645dbcc69 to b3c8597df0e2 (5 revisions) (flutter/flutter#141175) 2024-01-09 [email protected] Roll Packages from 5aef49b to 09205ca (2 revisions) (flutter/flutter#141173) 2024-01-09 [email protected] [Android] Add custom system-wide text selection toolbar buttons for SelectableRegion (flutter/flutter#141103) 2024-01-09 [email protected] Update Chips and `ChipTheme` tests and for Material 3 (flutter/flutter#141022) 2024-01-09 [email protected] Update `chip_test.dart` tests for Material 3 (flutter/flutter#140964) 2024-01-09 [email protected] Fix spell check throws when text contains regex reserved characters (flutter/flutter#140384) 2024-01-09 [email protected] Native assets support for Android Add2app (flutter/flutter#140802) 2024-01-09 [email protected] Roll Flutter Engine from d13479968b8f to 820645dbcc69 (3 revisions) (flutter/flutter#141152) 2024-01-09 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.22.11 to 3.23.0 (flutter/flutter#141132) 2024-01-09 [email protected] Shift mac Pixel 7 Pro tests to prod (flutter/flutter#141142) 2024-01-09 [email protected] Roll Flutter Engine from 3f0f8913169e to d13479968b8f (2 revisions) (flutter/flutter#141147) 2024-01-09 [email protected] Roll Flutter Engine from 09a74a08c819 to 3f0f8913169e (2 revisions) (flutter/flutter#141146) 2024-01-09 [email protected] Roll Flutter Engine from ee1ded6d1c44 to 09a74a08c819 (3 revisions) (flutter/flutter#141144) 2024-01-08 [email protected] [docs] Add document content related to chip shape (flutter/flutter#140015) 2024-01-08 [email protected] Roll Flutter Engine from 00ff33274fc0 to ee1ded6d1c44 (2 revisions) (flutter/flutter#141133) 2024-01-08 [email protected] Roll Flutter Engine from 70c45157c60a to 00ff33274fc0 (2 revisions) (flutter/flutter#141128) 2024-01-08 [email protected] Roll Flutter Engine from b57c899aefd7 to 70c45157c60a (1 revision) (flutter/flutter#141125) 2024-01-08 [email protected] Roll Flutter Engine from db564ffb3549 to b57c899aefd7 (1 revision) (flutter/flutter#141114) 2024-01-08 [email protected] Roll Packages from b9b6d38 to 5aef49b (2 revisions) (flutter/flutter#141117) 2024-01-08 [email protected] Roll Flutter Engine from bbebee146508 to db564ffb3549 (5 revisions) (flutter/flutter#141112) 2024-01-08 [email protected] Disable test shuffling in widget_tester_leaks_test.dart (flutter/flutter#141110) 2024-01-08 [email protected] Roll Flutter Engine from 702ae19c0026 to bbebee146508 (1 revision) (flutter/flutter#141088) 2024-01-08 [email protected] Roll Flutter Engine from 5e1d18bb51d1 to 702ae19c0026 (1 revision) (flutter/flutter#141084) 2024-01-07 [email protected] Roll Flutter Engine from 845ffd16d228 to 5e1d18bb51d1 (1 revision) (flutter/flutter#141076) 2024-01-07 [email protected] Roll Flutter Engine from ffc84af47827 to 845ffd16d228 (1 revision) (flutter/flutter#141075) 2024-01-07 [email protected] Roll Flutter Engine from cc9362752996 to ffc84af47827 (2 revisions) (flutter/flutter#141074) 2024-01-06 [email protected] Roll Flutter Engine from a5173fe3a912 to cc9362752996 (1 revision) (flutter/flutter#141066) 2024-01-06 [email protected] Roll Flutter Engine from 1dfcb6967dd1 to a5173fe3a912 (1 revision) (flutter/flutter#141063) 2024-01-06 [email protected] Roll Flutter Engine from 2fd75c06e0bd to 1dfcb6967dd1 (1 revision) (flutter/flutter#141062) 2024-01-06 [email protected] Roll Flutter Engine from 7f872b265733 to 2fd75c06e0bd (1 revision) (flutter/flutter#141059) 2024-01-06 [email protected] Roll Flutter Engine from 8d1789a43319 to 7f872b265733 (1 revision) (flutter/flutter#141056) 2024-01-06 [email protected] Roll Flutter Engine from fcb40168f159 to 8d1789a43319 (1 revision) (flutter/flutter#141053) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md ...

github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Dec 13, 2023

asci-00 mentioned this pull request Dec 14, 2023

3.16.2: Chip side: BordeSide.none does not work via ChipThemeData #139572

Closed

2 tasks

HansMuller self-requested a review December 15, 2023 22:39

HansMuller approved these changes Jan 8, 2024

View reviewed changes

asci-00 added 2 commits January 8, 2024 09:19

Added docs to chip

11632f3

fix: change how a property is referenced

f92f3d1

HansMuller force-pushed the fix/chip-api-docs-update branch from 39eda39 to f92f3d1 Compare January 8, 2024 17:19

Piinks approved these changes Jan 8, 2024

View reviewed changes

Piinks added d: api docs Issues with https://api.flutter.dev/ autosubmit Merge PR when tree becomes green via auto submit App labels Jan 8, 2024

cbracken approved these changes Jan 8, 2024

View reviewed changes

auto-submit bot merged commit 3b7ed00 into flutter:master Jan 8, 2024

asci-00 deleted the fix/chip-api-docs-update branch January 9, 2024 04:58

engine-flutter-autoroll mentioned this pull request Jan 9, 2024

Roll Flutter from c6ddb73f971e to 5f17badf4118 (30 revisions) flutter/packages#5850

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 9, 2024

3b7ed00 [docs] Add document content related to chip shape (flutter/fl…

6bff1cc

…utter#140015)

engine-flutter-autoroll mentioned this pull request Jan 9, 2024

Roll Flutter from c6ddb73f971e to d2d32b582f49 (31 revisions) flutter/packages#5851

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 9, 2024

3b7ed00 [docs] Add document content related to chip shape (flutter/fl…

0cd1ba1

…utter#140015)

engine-flutter-autoroll mentioned this pull request Jan 9, 2024

Roll Flutter from c6ddb73f971e to 126302dfb44c (32 revisions) flutter/packages#5852

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 9, 2024

3b7ed00 [docs] Add document content related to chip shape (flutter/fl…

5a04ef9

…utter#140015)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[docs] Add document content related to chip shape #140015

[docs] Add document content related to chip shape #140015

Uh oh!

asci-00 commented Dec 13, 2023 •

edited

Loading

Uh oh!

google-cla bot commented Dec 13, 2023

Uh oh!

flutter-dashboard bot commented Jan 4, 2024

Uh oh!

HansMuller left a comment

Uh oh!

Piinks left a comment

Uh oh!

cbracken left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

[docs] Add document content related to chip shape #140015

[docs] Add document content related to chip shape #140015

Uh oh!

Conversation

asci-00 commented Dec 13, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

google-cla bot commented Dec 13, 2023

Uh oh!

flutter-dashboard bot commented Jan 4, 2024

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

Piinks left a comment

Choose a reason for hiding this comment

Uh oh!

cbracken left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

asci-00 commented Dec 13, 2023 •

edited

Loading