Feat/math keyboard redesign theme a11y - #90
Conversation
Restyle the on-screen keyboard to the simpleclub design and add a
configurable theme plus first-class accessibility, keeping the public
API backwards compatible.
Theming:
- MathKeyboardStyle + MathKeyboardTheme inherited widget with a
Figma-default fallback, plumbed through MathField/MathFormField.
- Per-tier key tokens: digits on the secondary surface, operators and
variables on the neutral filled-button surface, delete on
secondary/weak, submit on primary; focus keeps the idle surface + ring.
- Function-key labels show example variables (x/y, x^2, sqrt{x}, ...)
instead of empty placeholders, matching the design.
Accessibility:
- Localizable MathKeyboardSemantics seam for all screen-reader strings,
including keyboard/section group labels.
- Keyboard is focus-reachable: tab hands focus from the field into the
keys, arrow keys traverse the grid, escape returns to the field, and
the keyboard stays open while either the field or a key holds focus.
- Keys expose single button nodes with spoken labels; sections are
wrapped as Variables/Functions/Numbers/Submit groups inside a top-level
"Math keyboard" group.
- Landscape focus order walks the number panel before the functions.
- Variable row shrinks keys past five so a partial key bleeds off the
edge, and auto-scrolls the focused item into view.
- Keep the keyboard open under a screen reader so the keys can be reached.
Fixes:
- Hover now works on web (driven by MouseRegion instead of the
highlight-mode-gated FocusableActionDetector callback).
Adds characterization and a11y tests covering behavior, focus, hover,
states, the variable bar, screen-reader survival, and semantic grouping.
The delete/back key stays on secondary/weak at idle but darkens to secondary/base on hover and pressed (matching the design). This also covers the page-toggle key, which shares the utility tier.
Rework the on-screen keyboard's focus model to the WAI-ARIA composite pattern and improve screen-reader access. Keyboard navigation: - The keyboard is a single tab stop: arrow keys move between all keys as a grid, Enter/Space activate, and tab leaves the keyboard for the next page control (shift+tab returns to the field) so focus is never trapped (WCAG 2.1.2). - Escape dismisses the keyboard and returns focus to the field without reopening it. Screen reader: - The field exposes an onTap semantics action so an accessibility activation (e.g. VoiceOver double-tap) opens the keyboard without a pointer. - The keyboard stays open while focus is on the keys so it can be reached. - Each section (variables, formula, numbers, submit) is a labelled landmark region, so a screen reader can jump straight to a section via landmark/rotor navigation. The keyboard wrapper stays a plain group. Other: - Rename the function keyboard's user-facing label to "Formula" (English fallback; consumers still inject their own translations). - Center the magnified label within the large-content-viewer overlay. Updates the focus, screen-reader, and semantics-group tests for the new model.
The redesign adds new API, a new dependency, and changes the default appearance, so bump the minor version and document it. Raise the SDK floor to Flutter 3.35.1 / Dart 3.9, required by SemanticsRole.region (used for the keyboard's section landmarks).
- Landscape submit is a single button, so wrap it only in a semantics boundary (to stay a sibling of the section landmarks) instead of a labelled region — a screen reader no longer announces "Submit" twice. - Remove the _keyboardFocusScopeNode listener before disposing it and guard its callback with a mounted check, matching the other teardown.
- Rephrase the public dartdoc for MathKeyboardStyle/fallback to drop simpleclub- and design-token-specific wording that is meaningless to third-party consumers. - Default MathKeyboardTheme.style to MathKeyboardStyle.fallback so a theme that only overrides semantics (for localization) no longer has to pass a full style.
Digits are on the function tier (secondary surface), not neutral; the neutral tier is operators, decimal, parentheses, cursor keys, and variables. Update the MathKeyboardKeyTier docs to match.
The inline comments on MathKeyboardStyle.fallback referenced internal design-token names (filledButton/neutral, secondary/base, primary/…), which are meaningless outside simpleclub. Describe each tier's keys and its idle/hover behaviour in neutral terms instead.
The Features list didn't mention the 0.4.0 additions. Add bullets for the configurable MathKeyboardTheme/MathKeyboardStyle, the accessibility support (landmarks, arrow-key navigation, localizable semantics), and the long-press large-text magnification, noting the large_content_viewer dependency it relies on.
The keys are ordinary buttons, so tab now traverses them one by one (the expected behaviour) rather than the keyboard being a single tab stop with arrows-only movement; the arrow keys stay as a bonus for 2-D grid movement. To avoid trapping focus, tab past the last key exits to the control after the field and shift+tab past the first key returns to the field, instead of wrapping within the keys (WCAG 2.1.2). Escape still closes.
Tab entered the on-screen keys in raw geometric order, zig-zagging across the two side-by-side landscape columns. Give each section an explicit FocusTraversalOrder so tab visits the number pad first, then the formula section, then submit, and switch the keyboard to an OrderedTraversalPolicy. The boundary-exit check uses the same policy so tab still leaves the keyboard at the ends instead of wrapping.
The section-level traversal order left keys inside a section to geometric reading order, which zig-zagged across the unevenly sized number cells. Assign each key an explicit FocusTraversalOrder under the single OrderedTraversalPolicy (numbers 0.., variables 100, functions 200.., submit 300) so tab follows the configured row-major layout end to end. Keeping the orders global (no nested groups) means the boundary-exit check still sees the true first and last key.
The page toggle is a mode switch, not content, so the long-press large-content magnifier adds no value there for a screen-reader user. Gate it on MediaQuery.accessibleNavigation: sighted users keep the long-press magnifier on the '123' key, screen-reader users don't.
|
|
|
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughThe 0.4.0 release adds keyboard theming, localized accessibility semantics, responsive landscape layouts, focus traversal, keyboard activation, large-content previews, and readable math-field descriptions. It also updates public exports, SDK requirements, documentation, and widget coverage. ChangesKeyboard redesign
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant ScreenReader
participant MathField
participant MathKeyboard
participant KeyboardButton
ScreenReader->>MathField: activate editable field
MathField->>MathKeyboard: open styled keyboard overlay
MathKeyboard->>KeyboardButton: expose semantic key and focus action
KeyboardButton->>MathField: submit key input
MathField->>ScreenReader: announce readable expression and cursor context
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Comment |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hi 👋🏽 Thank you for opening your first PR with simpleclub/math_keyboard ❤
You can expect a review from us soon
In the meantime, please check our contribution guidelines, the PR checklist, and the PR checks.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
math_keyboard/lib/src/widgets/math_keyboard.dart (1)
508-589: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winOff-screen variable keys are unreachable by keyboard and switch access.
ListView.separatedbuilds items lazily. Variables scrolled beyond the viewport and the cache extent have no element and therefore no focus node, so tab traversal and switch access skip them permanently. TheScrollable.ensureVisiblecallback only helps for keys that already exist. The row is explicitly sized so the sixth key bleeds off the edge, so this state is the normal case when a consumer passes more than five variables.Build all variable keys eagerly in a horizontal scroll view. The variable count is small, so the lazy build gives no measurable benefit.
♿ Proposed fix: build all keys eagerly
- return ListView.separated( - itemCount: variables.length, - scrollDirection: Axis.horizontal, - padding: EdgeInsets.only(left: leftPadding), - separatorBuilder: (context, index) => - SizedBox(width: style.rowSpacing), - itemBuilder: (context, index) { - return SizedBox( + return SingleChildScrollView( + scrollDirection: Axis.horizontal, + padding: EdgeInsets.only(left: leftPadding), + child: Row( + children: [ + for (var index = 0; index < variables.length; index++) ...[ + if (index > 0) SizedBox(width: style.rowSpacing), + SizedBox( width: keyWidth, child: Builder( builder: (itemContext) => KeyboardButton( // ... unchanged ), ), - ); - }, + ), + ], + ], + ), );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/lib/src/widgets/math_keyboard.dart` around lines 508 - 589, Replace the lazy ListView.separated in the build method with an eager horizontal scroll view that builds every variable key, preserving the existing spacing, sizing, focus handling, and scrolling behavior. Ensure all entries in variables have elements and focus nodes available for keyboard and switch-access traversal, including variables beyond the initial viewport.
🧹 Nitpick comments (8)
math_keyboard/test/widgets/math_cursor_context_test.dart (1)
10-65: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueDispose the controllers for consistency with the other test files.
Every other test file in this PR registers
addTearDown(controller.dispose). These six tests create aMathFieldEditingControllerand never dispose it. Add the teardown so the tests stay correct if leak tracking is enabled later.♻️ Example for the first test
final controller = MathFieldEditingController() ..addLeaf('7') ..addLeaf('8'); + addTearDown(controller.dispose);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/test/widgets/math_cursor_context_test.dart` around lines 10 - 65, Add addTearDown(controller.dispose) to each of the six tests after creating the MathFieldEditingController, including the controllers configured with leaves or functions. Keep the existing assertions and controller setup unchanged.math_keyboard/test/widgets/math_keyboard_focus_test.dart (1)
91-156: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAdd coverage for the backward exit path.
The tests cover forward Tab traversal and the forward exit through
onExitToNext. The backward boundary is untested:PreviousFocusIntentat the first key must invokeonExitToFieldand return focus to theMathFieldinstead of wrapping to the last key. Add a test that tabs into the keys once, then sends shift+Tab, and asserts that no key is focused and the field holds focus.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/test/widgets/math_keyboard_focus_test.dart` around lines 91 - 156, The existing focus traversal test only verifies forward exit; add a backward-boundary test near the current tab traversal coverage that tabs from the MathField into the keyboard once, then sends Shift+Tab and pumps focus updates. Assert that the MathField regains focus and no KeyboardButton remains focused, confirming PreviousFocusIntent uses onExitToField rather than wrapping to the last key.math_keyboard/test/widgets/math_keyboard_a11y_test.dart (1)
302-313: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueRename the misleading
germanvariable.
germanholdsMathKeyboardSemantics.fallback, which is the English default, and the overrides that follow are not a full German localization (variableLabelstill returns'Variable $name'). Rename it tobaseorenglish.♻️ Proposed rename
- const german = MathKeyboardSemantics.fallback; - final localized = german.copyWith( + const base = MathKeyboardSemantics.fallback; + final localized = base.copyWith( deleteLabel: 'Löschen', submitLabel: 'Bestätigen', variableLabel: (name) => 'Variable $name', );🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/test/widgets/math_keyboard_a11y_test.dart` around lines 302 - 313, Rename the local variable `german` in the `MathKeyboardSemantics override` test to `base` or `english`, since it references `MathKeyboardSemantics.fallback` rather than a German localization; update its subsequent `copyWith` usage accordingly.math_keyboard/lib/src/widgets/math_field.dart (3)
451-462: 🎯 Functional Correctness | 🔵 Trivial | ⚡ Quick winThe open keyboard does not pick up later theme changes.
The
OverlayEntrybuilder resolvesstyleandsemanticsfromthis.context, but nothing callsmarkNeedsBuildon the entry when the ancestorMathKeyboardThemechanges. An open keyboard therefore keeps the style and the semantics strings that were current when it opened. This also affects a locale change that swaps the semantics strings.Rebuild the entry when the field's dependencies change.
♻️ Proposed fix
+ `@override` + void didChangeDependencies() { + super.didChangeDependencies(); + // The overlay resolves the theme from this context, so it must rebuild + // when the theme or the locale changes. + _overlayEntry?.markNeedsBuild(); + }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/lib/src/widgets/math_field.dart` around lines 451 - 462, Update the widget lifecycle around the overlay entry and its builder so an open keyboard entry is marked for rebuild when the field's inherited theme or locale dependencies change. Ensure the refreshed builder re-resolves style and semantics from the current context, while preserving the existing overlay behavior.
988-1010: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low value
readableExpressionruns on every caret blink.
_FieldPreviewis rebuilt each time_cursorOpacitychanges, which happens twice per second while the field is focused. Each rebuild walks the whole expression tree and allocates a new string, even though the expression did not change. Cache the result and invalidate it innotifyListeners, or compute it only when the semantics value is actually requested.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/lib/src/widgets/math_field.dart` around lines 988 - 1010, Cache the readable expression used by readableExpression so repeated caret-blink rebuilds do not traverse the tree or allocate a new string when the expression is unchanged. Store the cached result alongside the semantics identity, reuse it when the same MathKeyboardSemantics is requested, and invalidate the cache in notifyListeners so edits and other field changes produce updated accessible text.
359-373: 🚀 Performance & Scalability | 🔵 Trivial | 💤 Low valueUse the aspect-scoped accessor.
MediaQuery.maybeAccessibleNavigationOfexists in Flutter 3.35.1 and avoids rebuilds for unrelatedMediaQueryDatachanges.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/lib/src/widgets/math_field.dart` around lines 359 - 373, Update _maybeCloseKeyboard to use MediaQuery.maybeAccessibleNavigationOf(context) instead of MediaQuery.maybeOf(context)?.accessibleNavigation, preserving the existing false fallback and keyboard-dismissal behavior.math_keyboard/lib/src/widgets/math_keyboard.dart (1)
900-921: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueTraversal-order numbers are magic literals with an implicit capacity limit.
numberKeyOrderstarts at0, variables are pinned to100,functionKeyOrderstarts at200, and submit is300. The scheme breaks silently iflandscapeNumberKeyboardever holds more than 100 keys orlandscapeFunctionKeyboardmore than 100 keys. Extract the range bases into named constants so the intent and the limit are explicit.Also applies to: 951-971
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/lib/src/widgets/math_keyboard.dart` around lines 900 - 921, Replace the traversal-order literals in the keyboard layout with named constants for the numbers, variables, functions, and submit range bases. Initialize numberKeyOrder and functionKeyOrder from the corresponding constants, and use the same constants for variable and submit ordering in the related section, making the 100-key range boundaries explicit.math_keyboard/test/widgets/math_keyboard_screen_reader_test.dart (1)
61-64: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueReplace the deprecated semantics-owner access.
Use
tester.semantics.performAction(find.bySemanticsLabel('Math field'), SemanticsAction.tap)and remove thegetSemanticscall and deprecation suppression.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@math_keyboard/test/widgets/math_keyboard_screen_reader_test.dart` around lines 61 - 64, In the screen-reader test, replace the deprecated tester.binding.pipelineOwner.semanticsOwner access and the intermediate node from getSemantics with tester.semantics.performAction targeting find.bySemanticsLabel('Math field') and SemanticsAction.tap; remove the deprecation suppression.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@math_keyboard/lib/src/foundation/math_keyboard_semantics.dart`:
- Around line 341-370: The hashCode implementation for the semantics class must
match mapEquals’ order-independent behavior. In the hash list near
functionMappings and tokenMappings, replace the ordered hashing of
tokenMappings.entries with Object.hashAllUnordered while preserving the existing
entry key/value hashes.
In `@math_keyboard/lib/src/widgets/math_field.dart`:
- Around line 314-330: Update the _suppressReopen branch in the focus-change
handler so it restarts _cursorBlinkController before returning, while preserving
the one-shot suppression and state update behavior.
- Around line 835-846: Update the Semantics wrapper in the math field widget to
expose the current decoration.errorText on the outer semantics node while
preserving the existing ExcludeSemantics behavior for the child InputDecorator.
Add a widget accessibility test covering MathFormField validation and assert
that the validator’s error text is present in the resulting semantics node.
In `@math_keyboard/lib/src/widgets/math_keyboard.dart`:
- Around line 636-639: Guard the page selection in _buildPortrait so
controller.secondPage cannot force-unwrap a null page2. When page2 is
unavailable, including number-only keyboards with secondPage set to true, fall
back to page1 or numberKeyboard while preserving page2 selection when it exists.
- Around line 118-137: Add an assertion in the MathKeyboardStyle constructor
requiring maxTextScaleFactor to be at least 1.0, so invalid values such as those
passed through copyWith are rejected at configuration time and the existing
clamp in the keyboard layout remains safe.
- Around line 1059-1066: Update the basic key semantics flow around
BasicKeyboardButtonConfig and _BasicButton to pass the button value and resolve
plain-text labels with semantics.tokenLabel(value) instead of the displayed
label. Ensure \frac receives a token mapping or dedicated division label, while
unmapped digit semantics remain unchanged.
---
Outside diff comments:
In `@math_keyboard/lib/src/widgets/math_keyboard.dart`:
- Around line 508-589: Replace the lazy ListView.separated in the build method
with an eager horizontal scroll view that builds every variable key, preserving
the existing spacing, sizing, focus handling, and scrolling behavior. Ensure all
entries in variables have elements and focus nodes available for keyboard and
switch-access traversal, including variables beyond the initial viewport.
---
Nitpick comments:
In `@math_keyboard/lib/src/widgets/math_field.dart`:
- Around line 451-462: Update the widget lifecycle around the overlay entry and
its builder so an open keyboard entry is marked for rebuild when the field's
inherited theme or locale dependencies change. Ensure the refreshed builder
re-resolves style and semantics from the current context, while preserving the
existing overlay behavior.
- Around line 988-1010: Cache the readable expression used by readableExpression
so repeated caret-blink rebuilds do not traverse the tree or allocate a new
string when the expression is unchanged. Store the cached result alongside the
semantics identity, reuse it when the same MathKeyboardSemantics is requested,
and invalidate the cache in notifyListeners so edits and other field changes
produce updated accessible text.
- Around line 359-373: Update _maybeCloseKeyboard to use
MediaQuery.maybeAccessibleNavigationOf(context) instead of
MediaQuery.maybeOf(context)?.accessibleNavigation, preserving the existing false
fallback and keyboard-dismissal behavior.
In `@math_keyboard/lib/src/widgets/math_keyboard.dart`:
- Around line 900-921: Replace the traversal-order literals in the keyboard
layout with named constants for the numbers, variables, functions, and submit
range bases. Initialize numberKeyOrder and functionKeyOrder from the
corresponding constants, and use the same constants for variable and submit
ordering in the related section, making the 100-key range boundaries explicit.
In `@math_keyboard/test/widgets/math_cursor_context_test.dart`:
- Around line 10-65: Add addTearDown(controller.dispose) to each of the six
tests after creating the MathFieldEditingController, including the controllers
configured with leaves or functions. Keep the existing assertions and controller
setup unchanged.
In `@math_keyboard/test/widgets/math_keyboard_a11y_test.dart`:
- Around line 302-313: Rename the local variable `german` in the
`MathKeyboardSemantics override` test to `base` or `english`, since it
references `MathKeyboardSemantics.fallback` rather than a German localization;
update its subsequent `copyWith` usage accordingly.
In `@math_keyboard/test/widgets/math_keyboard_focus_test.dart`:
- Around line 91-156: The existing focus traversal test only verifies forward
exit; add a backward-boundary test near the current tab traversal coverage that
tabs from the MathField into the keyboard once, then sends Shift+Tab and pumps
focus updates. Assert that the MathField regains focus and no KeyboardButton
remains focused, confirming PreviousFocusIntent uses onExitToField rather than
wrapping to the last key.
In `@math_keyboard/test/widgets/math_keyboard_screen_reader_test.dart`:
- Around line 61-64: In the screen-reader test, replace the deprecated
tester.binding.pipelineOwner.semanticsOwner access and the intermediate node
from getSemantics with tester.semantics.performAction targeting
find.bySemanticsLabel('Math field') and SemanticsAction.tap; remove the
deprecation suppression.
🪄 Autofix
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: e988c140-8c69-4fa0-98a1-b8919e7cc04d
📒 Files selected for processing (17)
math_keyboard/CHANGELOG.mdmath_keyboard/README.mdmath_keyboard/lib/math_keyboard.dartmath_keyboard/lib/src/foundation/keyboard_button.dartmath_keyboard/lib/src/foundation/math_keyboard_semantics.dartmath_keyboard/lib/src/widgets/keyboard_button.dartmath_keyboard/lib/src/widgets/math_field.dartmath_keyboard/lib/src/widgets/math_form_field.dartmath_keyboard/lib/src/widgets/math_keyboard.dartmath_keyboard/lib/src/widgets/math_keyboard_theme.dartmath_keyboard/pubspec.yamlmath_keyboard/test/widgets/math_cursor_context_test.dartmath_keyboard/test/widgets/math_keyboard_a11y_test.dartmath_keyboard/test/widgets/math_keyboard_focus_test.dartmath_keyboard/test/widgets/math_keyboard_screen_reader_test.dartmath_keyboard/test/widgets/math_keyboard_semantics_group_test.dartmath_keyboard/test/widgets/math_keyboard_test.dart
| int get hashCode => Object.hashAll([ | ||
| deleteLabel, | ||
| submitLabel, | ||
| showNumbersKeyboardLabel, | ||
| showFunctionsKeyboardLabel, | ||
| keyboardGroupLabel, | ||
| variablesGroupLabel, | ||
| functionsGroupLabel, | ||
| numbersGroupLabel, | ||
| moveCursorLeftLabel, | ||
| moveCursorRightLabel, | ||
| mathFieldLabel, | ||
| emptyLabel, | ||
| expressionContainerLabel, | ||
| numeratorLabel, | ||
| denominatorLabel, | ||
| exponentLabel, | ||
| rootIndexLabel, | ||
| underRootLabel, | ||
| underSquareRootLabel, | ||
| logBaseLabel, | ||
| logArgumentLabel, | ||
| insideParenthesesLabel, | ||
| variableLabel, | ||
| beforeToken, | ||
| startOfContainer, | ||
| endOfContainer, | ||
| Object.hashAll(functionMappings), | ||
| Object.hashAll( | ||
| tokenMappings.entries.map((e) => Object.hash(e.key, e.value))), |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
🧩 Analysis chain
🌐 Web query:
For Dart SDK 3.9, verify that Object.hashAllUnordered is available and is appropriate for hashing a Map's entries when equality is insertion-order independent.
💡 Result:
Object.hashAllUnordered is a long-standing member of the dart:core library, available in Dart SDK 3.9 and all stable versions since its introduction in Dart 2.14 [1][2][3]. It is appropriate to use Object.hashAllUnordered for hashing a Map's entries when equality is insertion-order independent [1][4]. Because the function computes a hash code by combining the hash codes of its input objects in an order-independent manner, it is well-suited for collections like Sets or Maps where the identity of the data does not depend on the specific sequence of the elements or entries [1][2][5]. When using this to hash a Map, you should pass the collection of Map entries (e.g., map.entries) to Object.hashAllUnordered [1]. Ensure that your equality logic also correctly handles unordered comparisons to maintain the required contract where equal objects must have identical hash codes [6][4][5].
Citations:
- 1: https://api.dart.dev/dart-core/Object/hashAllUnordered.html
- 2: https://github.com/dart-lang/sdk/blob/c0cc7a352a4e546e652a0ce9fb4be665c880e2c8/sdk/lib/core/object.dart
- 3: https://github.com/dart-lang/sdk/blob/efd81da467c5cfeaa39652bd865ce91830a66ab7/sdk/lib/core/object.dart
- 4: https://github.com/dart-lang/site-www/blob/main/src/content/libraries/dart-core.md
- 5: https://zetcode.com/dart/hashcode-equals/
- 6: https://dart.dev/libraries/dart-core
🏁 Script executed:
#!/bin/bash
set -e
file="$(fd -t f 'math_keyboard_semantics\.dart$' . | head -n 1)"
printf '%s\n' "FILE=$file"
sed -n '300,380p' "$file"
printf '%s\n' '--- equality and mapping references ---'
rg -n -C 4 'operator ==|mapEquals|tokenMappings|functionMappings|hashAllUnordered|environment|sdk' "$file" pubspec.yaml math_keyboard/pubspec.yaml 2>/dev/null || trueRepository: simpleclub/math_keyboard
Length of output: 15429
Use an order-independent hash for tokenMappings.
mapEquals ignores insertion order, but the current hash depends on tokenMappings.entries iteration order. Replace Object.hashAll with Object.hashAllUnordered for these entry hashes.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math_keyboard/lib/src/foundation/math_keyboard_semantics.dart` around lines
341 - 370, The hashCode implementation for the semantics class must match
mapEquals’ order-independent behavior. In the hash list near functionMappings
and tokenMappings, replace the ordered hashing of tokenMappings.entries with
Object.hashAllUnordered while preserving the existing entry key/value hashes.
| if (open) { | ||
| // Escape returns focus to the field but must leave the keyboard closed, | ||
| // so honor a one-shot suppression instead of reopening. | ||
| if (_suppressReopen) { | ||
| _suppressReopen = false; | ||
| setState(() {}); | ||
| return; | ||
| } | ||
| // Guard against re-opening while already shown: focus can return to the | ||
| // field from a key (e.g. via shift+tab), and re-inserting the overlay | ||
| // would restart the slide and drop the key focus. | ||
| if (!_isKeyboardShown) { | ||
| _openKeyboard(context); | ||
| _keyboardSlideController.forward(from: 0); | ||
| } | ||
| _cursorBlinkController.repeat(); | ||
|
|
||
| _showFieldOnScreen(); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
The caret stops blinking after Escape while the field keeps focus.
The _suppressReopen branch returns before _cursorBlinkController.repeat(). _handleFocusChanged(open: false) previously set the controller to 1 / 2, and _handleBlinkUpdate maps that value to opacity 0. After Escape the field holds focus but shows no caret, so the user loses the focus indication until the next tap.
🐛 Proposed fix
if (_suppressReopen) {
_suppressReopen = false;
+ // The field keeps focus, so the caret must stay visible.
+ _cursorBlinkController.repeat();
setState(() {});
return;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| if (open) { | |
| // Escape returns focus to the field but must leave the keyboard closed, | |
| // so honor a one-shot suppression instead of reopening. | |
| if (_suppressReopen) { | |
| _suppressReopen = false; | |
| setState(() {}); | |
| return; | |
| } | |
| // Guard against re-opening while already shown: focus can return to the | |
| // field from a key (e.g. via shift+tab), and re-inserting the overlay | |
| // would restart the slide and drop the key focus. | |
| if (!_isKeyboardShown) { | |
| _openKeyboard(context); | |
| _keyboardSlideController.forward(from: 0); | |
| } | |
| _cursorBlinkController.repeat(); | |
| _showFieldOnScreen(); | |
| if (open) { | |
| // Escape returns focus to the field but must leave the keyboard closed, | |
| // so honor a one-shot suppression instead of reopening. | |
| if (_suppressReopen) { | |
| _suppressReopen = false; | |
| // The field keeps focus, so the caret must stay visible. | |
| _cursorBlinkController.repeat(); | |
| setState(() {}); | |
| return; | |
| } | |
| // Guard against re-opening while already shown: focus can return to the | |
| // field from a key (e.g. via shift+tab), and re-inserting the overlay | |
| // would restart the slide and drop the key focus. | |
| if (!_isKeyboardShown) { | |
| _openKeyboard(context); | |
| _keyboardSlideController.forward(from: 0); | |
| } | |
| _cursorBlinkController.repeat(); | |
| _showFieldOnScreen(); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math_keyboard/lib/src/widgets/math_field.dart` around lines 314 - 330, Update
the _suppressReopen branch in the focus-change handler so it restarts
_cursorBlinkController before returning, while preserving the one-shot
suppression and state update behavior.
| return Semantics( | ||
| textField: true, | ||
| label: _semanticsLabel, | ||
| value: controller.readableExpression(semantics), | ||
| onTap: onTap, | ||
| child: ExcludeSemantics(child: field), | ||
| ); | ||
| } | ||
|
|
||
| /// The accessibility label describing the field itself. | ||
| String get _semanticsLabel => | ||
| decoration.labelText ?? decoration.hintText ?? semantics.mathFieldLabel; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
ExcludeSemantics hides the validation error from screen readers.
The wrapper excludes the whole InputDecorator subtree, which includes decoration.errorText. MathFormField sets errorText from the validator (see math_keyboard/lib/src/widgets/math_form_field.dart), so a failed validation is now rendered visually but is absent from the semantics tree. A screen-reader user receives no validation feedback.
Expose the error on the outer node.
♿ Proposed fix
return Semantics(
textField: true,
label: _semanticsLabel,
value: controller.readableExpression(semantics),
+ // The decorator subtree is excluded, so the validation error has to be
+ // carried by this node.
+ hint: decoration.errorText,
onTap: onTap,
child: ExcludeSemantics(child: field),
);Add a test that asserts the error text reaches the semantics node.
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return Semantics( | |
| textField: true, | |
| label: _semanticsLabel, | |
| value: controller.readableExpression(semantics), | |
| onTap: onTap, | |
| child: ExcludeSemantics(child: field), | |
| ); | |
| } | |
| /// The accessibility label describing the field itself. | |
| String get _semanticsLabel => | |
| decoration.labelText ?? decoration.hintText ?? semantics.mathFieldLabel; | |
| return Semantics( | |
| textField: true, | |
| label: _semanticsLabel, | |
| value: controller.readableExpression(semantics), | |
| // The decorator subtree is excluded, so the validation error has to be | |
| // carried by this node. | |
| hint: decoration.errorText, | |
| onTap: onTap, | |
| child: ExcludeSemantics(child: field), | |
| ); | |
| } | |
| /// The accessibility label describing the field itself. | |
| String get _semanticsLabel => | |
| decoration.labelText ?? decoration.hintText ?? semantics.mathFieldLabel; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math_keyboard/lib/src/widgets/math_field.dart` around lines 835 - 846, Update
the Semantics wrapper in the math field widget to expose the current
decoration.errorText on the outer semantics node while preserving the existing
ExcludeSemantics behavior for the child InputDecorator. Add a widget
accessibility test covering MathFormField validation and assert that the
validator’s error text is present in the resulting semantics node.
| final style = this.style ?? MathKeyboardTheme.styleOf(context); | ||
| final semantics = this.semantics ?? MathKeyboardTheme.semanticsOf(context); | ||
| final scaler = MediaQuery.textScalerOf(context); | ||
| final textScale = (scaler.scale(style.baseFontSize) / style.baseFontSize) | ||
| .clamp(1.0, style.maxTextScaleFactor); | ||
| final fontSize = style.baseFontSize * textScale; | ||
| // Keys default to a fixed size (maxTextScaleFactor == 1): large-text | ||
| // accessibility is provided by the large-content-viewer (long-press a key | ||
| // to magnify it), so the layout does not need to reflow. A consumer can | ||
| // instead opt into resize (WCAG 1.4.4) by raising maxTextScaleFactor, in | ||
| // which case keys grow up to keyHeight * maxTextScaleFactor. The FittedBox | ||
| // on each label stays as a safety net against overflow. | ||
| final keyHeight = style.keyHeight * textScale; | ||
| // In landscape the expression keyboard shows the functions and numbers side | ||
| // by side with a full-height submit key, instead of paging. The number-only | ||
| // keyboard keeps its compact paged layout. | ||
| final isLandscape = | ||
| type != MathKeyboardType.numberOnly && | ||
| MediaQuery.orientationOf(context) == Orientation.landscape; | ||
|
|
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard against a maxTextScaleFactor below 1.
clamp(1.0, style.maxTextScaleFactor) throws ArgumentError when maxTextScaleFactor is less than 1.0. MathKeyboardStyle accepts any double for this field, so a consumer can reach this crash through copyWith(maxTextScaleFactor: 0.8). Add an assertion on the field in MathKeyboardStyle (in math_keyboard/lib/src/widgets/math_keyboard_theme.dart), or clamp the upper limit here.
🛡️ Defensive fix at the consumption point
- final textScale = (scaler.scale(style.baseFontSize) / style.baseFontSize)
- .clamp(1.0, style.maxTextScaleFactor);
+ final maxTextScale = math.max(1.0, style.maxTextScaleFactor);
+ final textScale = (scaler.scale(style.baseFontSize) / style.baseFontSize)
+ .clamp(1.0, maxTextScale);The preferred fix is an assertion in the MathKeyboardStyle constructor so the misconfiguration is reported at its source.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math_keyboard/lib/src/widgets/math_keyboard.dart` around lines 118 - 137, Add
an assertion in the MathKeyboardStyle constructor requiring maxTextScaleFactor
to be at least 1.0, so invalid values such as those passed through copyWith are
rejected at configuration time and the existing clamp in the keyboard layout
remains safe.
| return AnimatedBuilder( | ||
| animation: controller, | ||
| builder: (context, child) { | ||
| final layout = controller.secondPage ? page2! : page1 ?? numberKeyboard; |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick win
Guard the page2! force-unwrap.
For MathKeyboardType.numberOnly, _buildPortrait passes page2: null. MathFieldEditingController.togglePage() is public and flips secondPage for any keyboard type, so a consumer can set secondPage to true on a number-only field. This build then throws a null-check error.
🛡️ Proposed guard
- final layout = controller.secondPage ? page2! : page1 ?? numberKeyboard;
+ final layout = (controller.secondPage ? page2 : page1) ?? numberKeyboard;📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| return AnimatedBuilder( | |
| animation: controller, | |
| builder: (context, child) { | |
| final layout = controller.secondPage ? page2! : page1 ?? numberKeyboard; | |
| return AnimatedBuilder( | |
| animation: controller, | |
| builder: (context, child) { | |
| final layout = (controller.secondPage ? page2 : page1) ?? numberKeyboard; |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math_keyboard/lib/src/widgets/math_keyboard.dart` around lines 636 - 639,
Guard the page selection in _buildPortrait so controller.secondPage cannot
force-unwrap a null page2. When page2 is unavailable, including number-only
keyboards with secondPage set to true, fall back to page1 or numberKeyboard
while preserving page2 selection when it exists.
| @override | ||
| Widget build(BuildContext context) { | ||
| final resolvedSemanticsLabel = | ||
| semanticsLabel ?? | ||
| (asTex | ||
| ? semantics.functionLabel(label!) | ||
| : (label == '.' ? decimalSeparator(context) : label)); | ||
|
|
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Inspect tokenLabel and its mappings to confirm the fallback behavior.
fd -t f 'math_keyboard_semantics.dart' | while IFS= read -r f; do
rg -n -C 6 'tokenLabel|tokenMappings' "$f"
doneRepository: simpleclub/math_keyboard
Length of output: 5010
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- math_keyboard.dart ---'
rg -n -C 12 'resolvedSemanticsLabel|class .*Key|final String\?? label|asTex' math_keyboard/lib/src/widgets/math_keyboard.dart
printf '%s\n' '--- math_keyboard_semantics.dart mappings ---'
rg -n -C 8 '_defaultTokenMappings|tokenMappings|[+×−=]' math_keyboard/lib/src/widgets/math_keyboard_semantics.dart
printf '%s\n' '--- math_field.dart reader ---'
rg -n -C 12 '_readNode|tokenLabel' math_keyboard/lib/src/widgets/math_field.dartRepository: simpleclub/math_keyboard
Length of output: 8288
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- candidate semantics files ---'
fd -t f -i 'semantics' math_keyboard
printf '%s\n' '--- math_field files ---'
fd -t f -i 'math_field' math_keyboard
printf '%s\n' '--- tokenLabel definitions and uses ---'
rg -n -C 8 'tokenLabel|_defaultTokenMappings|tokenMappings' math_keyboardRepository: simpleclub/math_keyboard
Length of output: 20492
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- button configuration definitions ---'
rg -n -C 10 'BasicKeyboardButtonConfig|KeyboardButtonConfig|label:|value:' math_keyboard/lib/src/widgets/math_keyboard.dart
printf '%s\n' '--- token-related tests ---'
rg -n -C 8 'tokenLabel|readableExpression|semanticsLabel|semantic' math_keyboard/testRepository: simpleclub/math_keyboard
Length of output: 23296
🏁 Script executed:
#!/bin/bash
set -e
printf '%s\n' '--- configuration classes and keyboard layouts ---'
rg -n -C 12 'class (BasicKeyboardButtonConfig|KeyboardButtonConfig)|BasicKeyboardButtonConfig\(|numberKeyboard|page1|page2|\\cdot|\\div|[×−]' math_keyboard/lib
printf '%s\n' '--- relevant accessibility assertions ---'
sed -n '186,250p' math_keyboard/test/widgets/math_keyboard_a11y_test.dart
sed -n '251,345p' math_keyboard/test/widgets/math_keyboard_a11y_test.dartRepository: simpleclub/math_keyboard
Length of output: 48591
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
import re
from pathlib import Path
semantics = Path("math_keyboard/lib/src/foundation/math_keyboard_semantics.dart").read_text()
keyboard = Path("math_keyboard/lib/src/foundation/keyboard_button.dart").read_text()
mapping_block = re.search(
r"_defaultTokenMappings\s*=\s*\{(.*?)\n\s*\};", semantics, re.S
).group(1)
mapping = dict(re.findall(r"\s*(r?'[^']*'|'[^']*')\s*:\s*'([^']*)'", mapping_block))
mapping = {k[1:] if k.startswith("r'") else k[1:]: v for k, v in mapping.items()}
pairs = re.findall(
r"label:\s*(r?'[^']*'|'[^']*'|\S+),\s*"
r"value:\s*(r?'[^']*'|'[^']*'|\S+)",
keyboard,
)
def decode(token):
if token.startswith(("r'", "'")) and token.endswith("'"):
return token[1:-1]
return token.rstrip(",")
def token_label(token):
return mapping.get(token, token.replace("{", "").replace("}", ""))
for raw_label, raw_value in pairs:
label, value = decode(raw_label), decode(raw_value)
if label in {"×", "÷", "−", "+", "(", ")"}:
print({
"label": label,
"value": value,
"tokenLabel(label)": token_label(label),
"tokenLabel(value)": token_label(value),
})
PYRepository: simpleclub/math_keyboard
Length of output: 672
🏁 Script executed:
#!/bin/bash
python3 - <<'PY'
import re
from pathlib import Path
semantics = Path("math_keyboard/lib/src/foundation/math_keyboard_semantics.dart").read_text()
keyboard = Path("math_keyboard/lib/src/foundation/keyboard_button.dart").read_text()
mapping_block = re.search(
r"_defaultTokenMappings\s*=\s*\{(.*?)\n\s*\};", semantics, re.S
).group(1)
mapping = {}
for key, value in re.findall(r"\s*(r?'[^']*'|'[^']*')\s*:\s*'([^']*)'", mapping_block):
mapping[key[2:-1] if key.startswith("r'") else key[1:-1]] = value
pairs = re.findall(
r"label:\s*(r?'[^']*'|'[^']*'|\S+),\s*"
r"value:\s*(r?'[^']*'|'[^']*'|\S+)",
keyboard,
)
def decode(token):
if token.startswith("r'") and token.endswith("'"):
return token[2:-1]
if token.startswith("'") and token.endswith("'"):
return token[1:-1]
return token.rstrip(",")
def token_label(token):
return mapping.get(token, token.replace("{", "").replace("}", ""))
for raw_label, raw_value in pairs:
label, value = decode(raw_label), decode(raw_value)
if label in {"×", "÷", "−", "+", "(", ")"}:
print(f"{label!r}: value={value!r}, "
f"tokenLabel(label)={token_label(label)!r}, "
f"tokenLabel(value)={token_label(value)!r}")
PYRepository: simpleclub/math_keyboard
Length of output: 619
Resolve plain-text key semantics from the inserted token.
The displayed × and − labels are not keys in tokenMappings; their inserted values are \cdot and -. Pass BasicKeyboardButtonConfig.value to _BasicButton, then call semantics.tokenLabel on that token. Add a mapping for \frac or provide a dedicated division label. Unmapped digits remain unchanged.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@math_keyboard/lib/src/widgets/math_keyboard.dart` around lines 1059 - 1066,
Update the basic key semantics flow around BasicKeyboardButtonConfig and
_BasicButton to pass the button value and resolve plain-text labels with
semantics.tokenLabel(value) instead of the displayed label. Ensure \frac
receives a token mapping or dedicated division label, while unmapped digit
semantics remain unchanged.
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #90 +/- ##
===========================================
+ Coverage 27.11% 72.51% +45.40%
===========================================
Files 11 13 +2
Lines 1147 1692 +545
===========================================
+ Hits 311 1227 +916
+ Misses 836 465 -371
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
…eading - MathField/MathFormField gain an optional `semanticsValue` that overrides the built-in expression linearization, so a consumer can inject speech from a dedicated TeX-to-speech engine. When null, the field falls back to `readableExpression`, keeping the package dependency-free and accessible on its own. - tokenLabel now speaks function-like leaves (e.g. the trig keys' `\cos(`) as words and strips any remaining TeX control chars, so a screen reader never reads out a raw backslash. - Guard the text-scale clamp with `math.max` so a sub-1 maxTextScaleFactor can't throw in release builds (asserts stripped). - A stray `togglePage()` on a number-only keyboard keeps the numbers layout and its region label in agreement (no null `page2` unwrap, no mislabel). - Skip the field's auto-scroll when the scroll view isn't attached, avoiding a "ScrollController not attached" assertion on rebuild.
The key tier now derives from asTex/isDigit, so BasicKeyboardButtonConfig.highlighted was read nowhere; the config class is unexported, so this has no consumer impact.
A bare MathKeyboard contains tab focus in its keys; focus only leaves via onExitToField/onExitToNext, which need focusScopeNode to work. Document that on the class and assert callbacks are never supplied without a scope (the silent case that never fires).
edhom
left a comment
edhom
left a comment
There was a problem hiding this comment.
LGTM. Love to see the math keyboard becoming accessible!
3 participants
Description
This PR redesigns the on-screen math keyboard and makes it themeable and accessible, while
keeping all existing input behavior intact.
Theming. The keyboard's appearance is now fully configurable through a new
MathKeyboardTheme/MathKeyboardStyleAPI — background, key colors (grouped into tiers:neutral, function, utility, primary), corner radius, spacing, and fonts. Wrap your subtree in a
MathKeyboardTheme, or pass astylestraight to aMathField. When no theme is present, abuilt-in default (dark) style is used, so existing users get a refreshed look with no code
changes.
MathKeyboardStyle/MathKeyboardKeyStyleare value types withcopyWith, sooverriding a single property is easy.
Accessibility. The keyboard is now usable with a physical keyboard, switch access, and a
screen reader:
keyboard at the ends without trapping focus (WCAG 2.1.2); the arrow keys additionally move
across the grid; escape dismisses the keyboard; and tab from the field steps onto the keys.
key is a labelled button, the cursor-navigation keys announce the new cursor context
(e.g. "before 8", "numerator", "end of expression"), and the keyboard's sections
(variables, formula, numbers, submit) are landmark regions the reader can jump between.
MathKeyboardSemanticsobject (English bydefault) that you can override to translate or adapt the announcements.
large_content_viewer), or opt into growing the keys with the system text scale.Landscape layout. In landscape the expression keyboard shows the functions and numbers side
by side with a dedicated full-height submit key, instead of paging.
Requirements & housekeeping. Uses
SemanticsRole.region, so the package now requiresFlutter 3.35.1 / Dart 3.9. Version bumped to
0.4.0with a matching CHANGELOG entry, and theREADME documents the theming, accessibility, and large-text features.
Related issues & PRs
theming API).
Checklist
contributing guide.
math_keyboardorexamplepackage(also README etc.), I created an entry in
CHANGELOG.md(## UPCOMING RELEASEif the changeon its own is not worth an update).
math_keyboardpackage, I updated the versionaccording to Dart's semantic versioning.