[Material] Add focus, highlight, and keyboard shortcuts to Slider by clocksmith · Pull Request #53945 · flutter/flutter

clocksmith · 2020-04-03T19:54:58Z

Description

Make the slider focusable and hoverable.
Create keyboard shortcuts to move the slider

Related Issues

closes #48905

Tests

I added the following tests:

slider.dart:

focus test
hover test
keyboard test

Checklist

Before you create this PR confirm that it meets all requirements listed below by checking the relevant checkboxes ([x]). This will ensure a smooth and quick review process.

I read the Contributor Guide and followed the process outlined there for submitting PRs.
I signed the CLA.
I read and followed the Flutter Style Guide, including Features we expect every widget to implement.
I read the Tree Hygiene wiki page, which explains my responsibilities.
I updated/added relevant documentation (doc comments with ///).
All existing and new tests are passing.
The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
I am willing to follow-up on review comments in a timely manner.

Breaking Change

Did any tests fail when you ran them? Please read Handling breaking changes.

No, no existing tests failed, so this is not a breaking change.
Yes, this is a breaking change. If not, delete the remainder of this section.
- I wrote a design doc: https://flutter.dev/go/template Replace this with a link to your design doc's short link
- I got input from the developer relations team, specifically from: Replace with the names of who gave advice
- I wrote a migration guide: Replace with a link to your migration guide

packages/flutter/test/material/slider_test.dart

…oard-slider

rami-a

LGTM with some questions/comments

packages/flutter/lib/src/material/slider.dart

packages/flutter/test/material/slider_theme_test.dart

packages/flutter/test/material/slider_test.dart

packages/flutter/lib/src/material/slider.dart

packages/flutter/lib/src/material/slider_theme.dart

…oard-slider

rami-a

LGTM

rami-a · 2020-04-09T17:37:44Z

packages/flutter/test/material/slider_test.dart

    ]);
  });
-}
+}


Seems like the new line got removed

gspencergoog

HansMuller

LGTM

…oard-slider

andraskende · 2020-04-16T16:34:31Z

I am trying to use slider in a video player on Android TV app, testing on Nvidia Shield TV
just upgraded master, with this change i can now use the slider with remote d-pad, but there is an issue:
slider once focused cant unfocus to other buttons on screen...
d-pad left and bottom rewinds, right and top forwards..... should be left: rewind, right forward,
top and bottom moves to other widgets...
here is my code:
https://github.com/andraskende/apptv1/blob/master/lib/videoplayer.dart#L202-L211
Thanks !!

gspencergoog · 2020-04-16T17:08:22Z

For text fields, we had to add the ignoreTextFields option to the DirectionalFocusIntent to make text fields allow DPAD movement vertically.

Perhaps what we really need is a UI mode that indicates that an app wants to use DPAD navigation instead of normal navigation modes.

gspencergoog · 2020-04-16T18:34:16Z

Actually, after discussing it with @goderbauer, I think I'm going to try and implement an InheritedWidget that sets the navigation mode for a subtree (much like Directionality does), as he suggested. Then we can use that to determine how to interpret arrow keys differently in cases where the navigation mode is DPAD-based.

clocksmith · 2020-04-16T18:58:27Z

@gspencergoog That sounds like a good idea. Do we have an idea of which components would need to update (and consume this InheritedWidget)?

@andraskende Can you open a separate ticket for adding proper DPAD support for the Slider?

gspencergoog · 2020-04-16T19:05:14Z

For various different reasons, I think this is the list:

Slider (for obvious reasons)
TextField to handle turning off of up/down cursor movement, similar to slider.
All focusable controls that can be disabled, for allowing focus while disabled (see Allow controls to remain focusable while in disabled state #54877).

gmoraja · 2020-04-21T01:54:34Z

@gspencergoog on a mobile device, the slider can be correctly navigated with an external keyboard now, but it doesn't receive focus with TalkBack yet.

Make slider focusable and hoverable, need to fix Semantics tests

1ea04d0

fluttergithubbot added f: material design flutter/packages/flutter/material repository. framework flutter/packages/flutter repository. See also f: labels. work in progress; do not review labels Apr 3, 2020

googlebot added the cla: yes label Apr 3, 2020

clocksmith added 4 commits April 3, 2020 15:59

update overlayColor doc and extract actionFactory

b33ea19

Remove print statements

8754592

Remove unneccessary markNeedsPaint calls

9d4e9e5

Remove commented out test code

edf1190

goderbauer reviewed Apr 3, 2020

View reviewed changes

packages/flutter/test/material/slider_test.dart Outdated Show resolved Hide resolved

clocksmith added 2 commits April 3, 2020 17:37

Fix tests

fbb8c95

Add platform specific keyboard tests

f3860f5

clocksmith marked this pull request as ready for review April 3, 2020 21:44

clocksmith changed the title ~~Make slider focusable and hoverable, need to fix Semantics tests~~ [Material] Add focus, highlight, and keyboard shortcuts to Slider Apr 3, 2020

clocksmith added 6 commits April 3, 2020 18:07

analyzer

0097174

Merge branch 'master' of https://github.com/flutter/flutter into keyb…

d30c6cc

…oard-slider

Fix tests

efbde9e

fix theme tests

63b88c8

fix theme tests

907b1d9

Fix tests

4897ba8

clocksmith removed the work in progress; do not review label Apr 6, 2020

rami-a approved these changes Apr 6, 2020

View reviewed changes

Rami comments

2b5961e

clocksmith requested a review from gspencergoog April 6, 2020 18:42

gspencergoog reviewed Apr 6, 2020

View reviewed changes

packages/flutter/lib/src/material/slider.dart Show resolved Hide resolved

packages/flutter/lib/src/material/slider_theme.dart Show resolved Hide resolved

clocksmith requested a review from HansMuller April 7, 2020 15:06

clocksmith added 4 commits April 7, 2020 22:41

Remove shift from keyboard shortcut

6ed088b

Refactor update logic for hover and focus

33f2452

merge

e31f960

Merge branch 'master' of https://github.com/flutter/flutter into keyb…

f5f4970

…oard-slider

clocksmith added 3 commits April 9, 2020 12:13

merge

d36f765

Fix tests

afedaaf

Reorder diagnostics properties to match constructor

a170213

rami-a approved these changes Apr 9, 2020

View reviewed changes

add new line

b89f375

gspencergoog approved these changes Apr 9, 2020

View reviewed changes

HansMuller approved these changes Apr 16, 2020

View reviewed changes

Merge branch 'master' of https://github.com/flutter/flutter into keyb…

b36b31b

…oard-slider

clocksmith added the waiting for tree to go green label Apr 16, 2020

fluttergithubbot merged commit b00b4e8 into flutter:master Apr 16, 2020

andraskende mentioned this pull request Apr 16, 2020

Slider DPAD support for Android TV #54984

Open

clocksmith deleted the keyboard-slider branch June 8, 2020 13:59

github-actions bot locked as resolved and limited conversation to collaborators Jul 31, 2021

                   ]);
                 });
-              }
+              }

                
                    No newline at end of file

Conversation

clocksmith commented Apr 3, 2020

Description

Related Issues

Tests

Checklist

Breaking Change

Uh oh!

Uh oh!

rami-a left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

rami-a left a comment

Choose a reason for hiding this comment

Uh oh!

rami-a Apr 9, 2020

Choose a reason for hiding this comment

Uh oh!

gspencergoog left a comment

Choose a reason for hiding this comment

Uh oh!

HansMuller left a comment

Choose a reason for hiding this comment

Uh oh!

andraskende commented Apr 16, 2020

Uh oh!

gspencergoog commented Apr 16, 2020

Uh oh!

gspencergoog commented Apr 16, 2020

Uh oh!

clocksmith commented Apr 16, 2020

Uh oh!

gspencergoog commented Apr 16, 2020

Uh oh!

gmoraja commented Apr 21, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

9 participants