Skip to content

Conversation

@DBowen33
Copy link
Contributor

@DBowen33 DBowen33 commented Aug 6, 2024

Fixed issue to where value indicator label is not shown to user when initially focused.

Before: Screen recording 2024-08-05 12.16.24 PM.webm
After: https://screencast.googleplex.com/cast/NTY5NzIzMTYxNjIxMjk5MnxkY2IyMGNkYi1iZA

Fixes #113538

Pre-launch Checklist

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

@github-actions github-actions bot added framework flutter/packages/flutter repository. See also f: labels. f: material design flutter/packages/flutter/material repository. labels Aug 6, 2024
@DBowen33 DBowen33 marked this pull request as ready for review August 6, 2024 21:29
@Piinks
Copy link
Contributor

Piinks commented Aug 7, 2024

FYI @TahaTesser in case this affects the slider rewrite (which I need to catch up on reviewing!)

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:) Thanks for the fix!

Comment on lines 4559 to 4563
theme: ThemeData(
sliderTheme: const SliderThemeData(
showValueIndicator: ShowValueIndicator.always,
),
),
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: Seems we can remove this part?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done!

final FocusNode focusNode = FocusNode();
addTearDown(focusNode.dispose);

Widget buildApp({bool enabled = true}) {
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: maybe we can remove the parameter here since we only tested the enable slider:)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done, thanks for the recs!

Copy link
Contributor

@MitchellGoodwin MitchellGoodwin left a comment

Choose a reason for hiding this comment

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

LGTM!

@DBowen33 DBowen33 force-pushed the slider-keyboard-focus-fix branch from 7d3e509 to fbe9704 Compare August 13, 2024 18:16
@Piinks Piinks added the autosubmit Merge PR when tree becomes green via auto submit App label Aug 14, 2024
@auto-submit auto-submit bot merged commit 51c6402 into flutter:master Aug 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Aug 15, 2024
flutter/flutter@cc13cd1...99f00a1

2024-08-15 [email protected] Roll Flutter Engine from 76a1c64f4e63 to 971ddd9fe1bf (6 revisions) (flutter/flutter#153487)
2024-08-14 [email protected] Update tokens to 5.0.0 (flutter/flutter#153385)
2024-08-14 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.26.1 to 3.26.2 (flutter/flutter#153468)
2024-08-14 [email protected] Enable invalid_runtime_check_with_js_interop_types, use_truncating_division (flutter/flutter#153464)
2024-08-14 [email protected] Roll Flutter Engine from 9b8421662ec9 to 76a1c64f4e63 (1 revision) (flutter/flutter#153465)
2024-08-14 [email protected] Roll Flutter Engine from 328c4df8329f to 9b8421662ec9 (1 revision) (flutter/flutter#153463)
2024-08-14 [email protected] Lint sync (flutter/flutter#153453)
2024-08-14 [email protected] manual pub roll w/ gradle updates (flutter/flutter#153403)
2024-08-14 [email protected] Update docs to use new Android version in the engine (flutter/flutter#151601)
2024-08-14 [email protected] move some tool TESTOWNERS entries to andrew (flutter/flutter#153400)
2024-08-14 [email protected] Slider shows visual label of value on focus (flutter/flutter#152960)
2024-08-14 [email protected] Add `Row` and `Column` widgets specific `spacing` tests (flutter/flutter#153353)
2024-08-14 [email protected] Delay `DropdownMenu` filtering until text input (flutter/flutter#152368)
2024-08-14 [email protected] Update cherry pick issue template's PR field (flutter/flutter#153381)
2024-08-14 [email protected] Roll Flutter Engine from 5909666fdedb to 328c4df8329f (1 revision) (flutter/flutter#153436)
2024-08-14 [email protected] [web] Pass `--no-source-maps` instead of `--extra-compiler-option=--no-source-maps` to `dart compile wasm` (flutter/flutter#153417)
2024-08-14 [email protected] Manual roll Flutter Engine from 019f9e3f0744 to 5909666fdedb (12 revisions) (flutter/flutter#153413)
2024-08-14 [email protected] Move `@_debugOnly` documentation in `framework.dart` to be more visible to IDE. (flutter/flutter#153134)
2024-08-13 [email protected] Roll Flutter Engine from 4246f1536c5d to 019f9e3f0744 (2 revisions) (flutter/flutter#153394)
2024-08-13 [email protected] fix(flutter/a11y assessments): h1 missing a11y from each page on the web app (flutter/flutter#152198)
2024-08-13 49699333+dependabot[bot]@users.noreply.github.com Bump github/codeql-action from 3.26.0 to 3.26.1 (flutter/flutter#153395)
2024-08-13 [email protected] Roll Packages from d9a6de8 to e4f2247 (4 revisions) (flutter/flutter#153383)
2024-08-13 [email protected] Roll pub packages (flutter/flutter#153380)
2024-08-13 [email protected] Roll Flutter Engine from 73c3135cc3be to 4246f1536c5d (2 revisions) (flutter/flutter#153377)

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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

autosubmit Merge PR when tree becomes green via auto submit App 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.

Slider does not show value indicator when focused.

4 participants