feat: add key modifiers to layout configuration#202
Conversation
|
Warning Review limit reached
Next review available in: 41 minutes Enable usage-based reviews in Billing to review now. Otherwise, wait until the next included review is available. How can I continue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based reviews. How do review limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please refer docs for additional details. Review details⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds typed key metadata for custom layouts, including tracked keys, top labels, and held states. Adds configurable long-key and top-label font sizes across state, preferences, rendering, serialization, tests, and documentation. ChangesKey Modifiers and Text Settings
Estimated code review effort: 4 (Complex) | ~45 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant TextTab
participant KeyboardNotifier
participant KeyboardState
participant KeyboardScreen
User->>TextTab: adjust font-size slider
TextTab->>KeyboardNotifier: update font size
KeyboardNotifier->>KeyboardState: copyWith updated sizing
KeyboardScreen->>KeyboardState: read sizing and key metadata
KeyboardScreen-->>User: render labels and keys
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (2)
lib/screens/keyboard_screen.dart (1)
212-217: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winCache
altLayoutKeyand reusekeyFontSizefor clarity and efficiency.You can simplify the widget tree and avoid invoking
_getAltLayoutKeymultiple times by computing thealtLayoutKeyupfront alongsidekeyFontSize. You can also reuse the pre-calculatedkeyFontSizevariable for the primary key'sTextwidget instead of duplicating the ternary check.♻️ Proposed refactor
final topLabel = keySpec?.topLabel; final topLabelFontSize = keyboardState.topLabelFontSize; + final altLayoutKey = altLayout != null + ? _getAltLayoutKey(rowIndex, keyIndex, keyboardState, prefsState, altLayout) + : null;Update the corresponding sections inside the
Stack:- if (altLayout != null) + if (altLayoutKey != null) Positioned( top: 4, left: 8, child: Text( key, textAlign: TextAlign.left, style: TextStyle( color: textColor, - fontSize: key.length > 2 - ? keyboardState.longKeyFontSize - : keyboardState.keyFontSize, + fontSize: keyFontSize, fontWeight: keyboardState.fontWeight, ), ), )And for the alternative layout key:
- if (altLayout != null) + if (altLayoutKey != null) Positioned( bottom: 4, right: 8, child: Text( - _getAltLayoutKey(rowIndex, keyIndex, keyboardState, - prefsState, altLayout), + altLayoutKey, textAlign: TextAlign.right, style: TextStyle( color: textColor, - fontSize: _getAltLayoutKey( - rowIndex, - keyIndex, - keyboardState, - prefsState, - altLayout) - .length > - 2 + fontSize: altLayoutKey.length > 2 ? keyboardState.longKeyFontSize : keyboardState.keyFontSize, fontWeight: keyboardState.fontWeight, ), ), ),Also applies to: 265-279, 310-333
🤖 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 `@lib/screens/keyboard_screen.dart` around lines 212 - 217, Within the keyboard widget-building method, cache the result of _getAltLayoutKey alongside hasTopLabel and keyFontSize, then reuse altLayoutKey wherever the alternative layout key is referenced instead of invoking the helper repeatedly. Update the primary key Text widget to use the existing keyFontSize variable, and apply the same reuse in the corresponding sections around the Stack and alternative-layout handling.lib/widgets/options/base_option.dart (1)
24-32: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueGuard
Platform.environmentchecks for web compatibility.While this application might primarily target desktop platforms, importing
dart:ioand directly accessingPlatformproperties will causeUnsupportedErrorexceptions or compilation failures if the UI is ever run in a web environment.As a best practice, consider guarding this with
kIsWebfromflutter/foundation.dart.🛠️ Proposed adjustment
Add the import at the top of the file:
+import 'package:flutter/foundation.dart'; import 'dart:io';And update the condition:
// Disable splash effects during widget tests to avoid engine shader // runtime issues (ink_sparkle.frag mismatches). Only override in test // environment so production behavior isn't changed. - if (Platform.environment.containsKey('FLUTTER_TEST')) { + if (!kIsWeb && Platform.environment.containsKey('FLUTTER_TEST')) { return Theme(🤖 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 `@lib/widgets/options/base_option.dart` around lines 24 - 32, Guard the FLUTTER_TEST environment check in the widget’s build logic using Flutter’s kIsWeb indicator, ensuring Platform.environment is never accessed on web. Add the foundation import and preserve the existing NoSplash Theme override for non-web test environments and normal container behavior otherwise.
🤖 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 `@lib/models/user_config.dart`:
- Around line 52-53: Update the trackedKey assignment in the key-mapping logic
to use an empty string when both the raw `t` value and topLabel are unavailable,
instead of falling back to rawKey.toString(). Preserve the existing precedence
for valid t values and topLabel.
In `@README.md`:
- Line 155: Update the documentation links in the README entries around “Key
modifiers” and “Text Settings” to use repository-relative paths such as docs/...
or ./docs/... instead of site-root-relative /docs/... paths. Normalize the moved
entries consistently while preserving their existing targets and labels.
---
Nitpick comments:
In `@lib/screens/keyboard_screen.dart`:
- Around line 212-217: Within the keyboard widget-building method, cache the
result of _getAltLayoutKey alongside hasTopLabel and keyFontSize, then reuse
altLayoutKey wherever the alternative layout key is referenced instead of
invoking the helper repeatedly. Update the primary key Text widget to use the
existing keyFontSize variable, and apply the same reuse in the corresponding
sections around the Stack and alternative-layout handling.
In `@lib/widgets/options/base_option.dart`:
- Around line 24-32: Guard the FLUTTER_TEST environment check in the widget’s
build logic using Flutter’s kIsWeb indicator, ensuring Platform.environment is
never accessed on web. Add the foundation import and preserve the existing
NoSplash Theme override for non-web test environments and normal container
behavior otherwise.
🪄 Autofix (Beta)
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: defaults
Review profile: CHILL
Plan: Pro
Run ID: cc0b7683-a774-4db7-96e1-80b31e16128f
⛔ Files ignored due to path filters (3)
docs/advanced/image-1.pngis excluded by!**/*.pngdocs/advanced/image.pngis excluded by!**/*.pngpubspec.lockis excluded by!**/*.lock
📒 Files selected for processing (14)
README.mddocs/advanced/key-modifiers.mddocs/index.mddocs/user-guide/preferences.mddocs/user-guide/text.mdlib/models/keyboard_layouts.dartlib/models/user_config.dartlib/providers/keyboard_provider.dartlib/screens/keyboard_screen.dartlib/services/method_call_handler.dartlib/widgets/options/base_option.dartlib/widgets/tabs/text_tab.darttest/providers/keyboard_provider_test.darttest/user_config_test.dart
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
This pull request introduces support for key modifiers in custom layouts, allowing keys to have a separate top label, a tracked key, and a visual held state. It also adds new text settings for long labels and top labels, updates the configuration and serialization logic, and provides comprehensive documentation for these features. The most important changes are grouped below.
Key Modifier Support in Custom Layouts:
KeyboardKeySpecclass to represent keys with a tracked key, optional top label, and held/active visual state (type: "held"). This enables richer custom layouts and modifier keys.UserConfigparsing to support both plain string keys and object-based keys with modifier fields (h,t,type). Serialization logic now preserves these objects. [1] [2]Text Settings Enhancements:
KeyboardState, with corresponding update methods and serialization support. [1] [2] [3] [4] [5] [6] [7]Documentation Updates:
docs/advanced/key-modifiers.mdanddocs/user-guide/text.mdto explain key modifier objects and text settings. [1] [2]README.mdanddocs/index.mdto reference the new key modifier and text settings documentation. [1] [2] [3] [4]These changes make custom layouts much more flexible and improve the appearance and usability of complex keyboard overlays.
Summary by CodeRabbit
New Features
Documentation
Tests