Skip to content

Adds semantics role checks - #162290

Merged
auto-submit[bot] merged 9 commits into
flutter:masterfrom
chunhtai:issues/161346
Jan 29, 2025
Merged

auto-submit[bot] merged 9 commits into
flutter:masterfrom
chunhtai:issues/161346

Conversation

@chunhtai

Copy link
Copy Markdown
Contributor

Adds test and error detection system for semantics roles.

Pre-launch Checklist

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

@chunhtai
chunhtai requested a review from hannah-hyj January 27, 2025 23:51
@github-actions github-actions Bot added framework flutter/packages/flutter repository. See also f: labels. a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) labels Jan 27, 2025

@hannah-hyj hannah-hyj left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM with some comments on tests

expect(SemanticsRole.values.toSet(), DebugSemanticsRoleChecks.kChecks.keys.toSet());
});

group('tab', () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

add a successful case too?

child: Semantics(role: SemanticsRole.tab, child: const Text('a tab')),
),
);
expect(tester.takeException(), isFlutterError);

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

can you also expect the error string here, so it will be more clear

});
});

group('tabBar', () {

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

ditto, add a successful case too

/// to [kChecks].
@visibleForTesting
class DebugSemanticsRoleChecks {
DebugSemanticsRoleChecks._();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Drive-by comment: this can be a final class ? Then DebugSemanticsRoleChecks._(); is not needed.

@chunhtai chunhtai Jan 28, 2025 •

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

final class can still be instantiated I think? I use sealed instead

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Yeah, you're right, I had to double check the class modifiers reference :)
But yeah, we should be able to eliminate the constructor anyway

@github-actions github-actions Bot added p: material_ui material_ui package in flutter/packages f: scrolling Viewports, list views, slivers, etc. labels Jan 28, 2025
@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 29, 2025
@auto-submit auto-submit Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 29, 2025
@auto-submit

auto-submit Bot commented Jan 29, 2025

Copy link
Copy Markdown
Contributor

autosubmit label was removed for flutter/flutter/162290, because - The status or check suite Google testing has failed. Please fix the issues identified (or deflake) before re-applying this label.

@chunhtai chunhtai added the autosubmit Merge PR when tree becomes green via auto submit App label Jan 29, 2025
@auto-submit
auto-submit Bot added this pull request to the merge queue Jan 29, 2025
Merged via the queue into flutter:master with commit 07ab4ad Jan 29, 2025
@flutter-dashboard flutter-dashboard Bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jan 29, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 30, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jan 31, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 1, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 2, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 3, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

a: accessibility Accessibility, e.g. VoiceOver or TalkBack. (aka a11y) f: scrolling Viewports, list views, slivers, etc. framework flutter/packages/flutter repository. See also f: labels. p: material_ui material_ui package in flutter/packages

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants