Skip to content

Add a Clarification for the docs of suggestionsBuilder of SearchAnchor#183106

Merged
auto-submit[bot] merged 1 commit intoflutter:masterfrom
ahmedsameha1:handle#183105
Mar 18, 2026
Merged

Add a Clarification for the docs of suggestionsBuilder of SearchAnchor#183106
auto-submit[bot] merged 1 commit intoflutter:masterfrom
ahmedsameha1:handle#183105

Conversation

@ahmedsameha1
Copy link
Contributor

@ahmedsameha1 ahmedsameha1 commented Mar 2, 2026

This is my attempt to handle: #183105.

Fixes #183105

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Mar 2, 2026
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request aims to clarify when the suggestionsBuilder is called. The added documentation is helpful but not entirely accurate. I've suggested a more precise wording to reflect that the builder is also called when the search view is first displayed, in addition to when the text changes.

final SearchAnchorChildBuilder builder;

/// Called to get the suggestion list for the search view.
/// It is called only when the search text changes.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

This clarification is helpful, but not entirely accurate. The suggestionsBuilder is also called when the search view is first displayed (with an empty search text), not just when the text changes subsequently. A more precise phrasing would be better.

  /// It is called when the search view is shown, and again whenever the search text changes.

QuncCccccc
QuncCccccc previously approved these changes Mar 4, 2026
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for your contribution:) LGTM.

final SearchAnchorChildBuilder builder;

/// Called to get the suggestion list for the search view.
/// After the search view is displayed, it is called only when the search text changes.
Copy link
Contributor

@QuncCccccc QuncCccccc Mar 4, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// After the search view is displayed, it is called only when the search text changes.
///
/// This builder is called once when the search view is first displayed,
/// and subsequently every time the search text changes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ahmedsameha1 Once we updated the doc with the suggestion, I'll ask for a second review and then we are good to go:)!

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I committed and pushed your suggestion. Do you think that viewBuilder needs a similar clarification? As it overrides suggestionsBuilder.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Running dart format packages/flutter/lib/src/material/search_anchor.dart will fix the failed Linux Analyze test.

As it overrides suggestionsBuilder.

I don't think it overrides suggestionsBuilder. The viewBuilder is used to override the default layout.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ok, I misunderstood it. I tested it, and the print statement in the viewBuilder did not appear to be called when the search text changes, but the one in the suggestionsBuilder prints on the console. Maybe I will ask for clarification for this in another issue!

@ahmedsameha1 ahmedsameha1 force-pushed the handle#183105 branch 2 times, most recently from 4e89138 to 6209ec6 Compare March 7, 2026 05:05
@github-actions github-actions bot removed the CICD Run CI/CD label Mar 17, 2026
Copy link
Contributor

@QuncCccccc QuncCccccc left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@QuncCccccc QuncCccccc added the CICD Run CI/CD label Mar 17, 2026
@dkwingsmt dkwingsmt added the autosubmit Merge PR when tree becomes green via auto submit App label Mar 17, 2026
@auto-submit auto-submit bot added this pull request to the merge queue Mar 17, 2026
Merged via the queue into flutter:master with commit aaefa67 Mar 18, 2026
79 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 18, 2026
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Mar 19, 2026
flutter/flutter@d117642...dd64978

2026-03-18 [email protected] Roll Skia from 2fb5fa71eb12 to f0a13e5efbad (2 revisions) (flutter/flutter#183830)
2026-03-18 [email protected] Roll Skia from ae3d36cb9e29 to 2fb5fa71eb12 (3 revisions) (flutter/flutter#183823)
2026-03-18 [email protected] Linux reuse sibling (flutter/flutter#183653)
2026-03-18 [email protected] Roll Skia from 84a180a1fa80 to ae3d36cb9e29 (4 revisions) (flutter/flutter#183812)
2026-03-18 [email protected] fix(web): handle asynchronously disposed platform views (flutter/flutter#183666)
2026-03-17 [email protected] (Test cross-imports) Remove legacy Material import from sliver_constraints_test (flutter/flutter#183351)
2026-03-17 [email protected] Fix Android Studio pluginsPath when version is unknown (do not use 0.0) (flutter/flutter#182681)
2026-03-17 [email protected] Fixes animation glitch into bottom sheet (flutter/flutter#183303)
2026-03-17 [email protected] Handle#6537 second grouped test (flutter/flutter#182529)
2026-03-17 [email protected] Add a Clarification for the docs of suggestionsBuilder of SearchAnchor (flutter/flutter#183106)
2026-03-17 [email protected] Remove obsolete null checks from style guide (flutter/flutter#181703)
2026-03-17 [email protected] [Impeller] Do not delete the GL object in a HandleGLES if the handle has a cleanup callback (flutter/flutter#183561)
2026-03-17 [email protected] Encode source file patches as UTF-8 in the code formatter script (flutter/flutter#183761)
2026-03-17 [email protected] Fix widget inspector control layout and add safe area regression test (flutter/flutter#180789)
2026-03-17 [email protected] Reland "[Android] Add mechanism for setting Android engine flags via Android manifest (take 2)" (flutter/flutter#182522)
2026-03-17 [email protected] fix(web): fix crash in Skwasm when transferring non-transferable texture sources (flutter/flutter#183799)
2026-03-17 [email protected] Roll Skia from dba893a44d7a to 84a180a1fa80 (7 revisions) (flutter/flutter#183803)
2026-03-17 [email protected] Framework: Improve DropdownButton selectedItemBuilder assertion (flutter/flutter#183732)
2026-03-17 [email protected] Add mainAxisAlignment to NavigationRail (flutter/flutter#183514)
2026-03-17 [email protected] Update android triage process to not look at unassigned p1s every week (flutter/flutter#183805)
2026-03-17 [email protected] Adds macos impeller new gallery transition perf test. (flutter/flutter#183802)
2026-03-17 [email protected] fix(web_ui): move prepareToDraw after raster to improve concurrency and stability (flutter/flutter#183791)
2026-03-17 [email protected] [build] Generate debug info for assembly. (flutter/flutter#183425)
2026-03-17 [email protected] [web] Fix occasional failure to find Chrome tab (flutter/flutter#183737)

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] 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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

CICD Run CI/CD f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clarify the docs of suggestionsBuilder of SearchAnchor

4 participants