RCF-1308: added unique id for local value editable text field in global config settings #666
Conversation
…al config settings Signed-off-by: sachin.sp <sachin.sp@cyberpwn.com>
WalkthroughAdded extensive accessibility and testability instrumentation to the global config settings tab: Semantics wrappers, ExcludeSemantics for raw text, and unique diagnostic keys for rows, keys, server/local values, and editable containers. Behavioral logic and public APIs remain unchanged. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Repository UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
lib/ui/settings/widgets/global_config_settings_tab.dart (2)
403-444: Fix contradictory Semantics configuration and improve accessibility labels.The Semantics implementation has several issues:
Contradictory settings: The parent
SemanticshasexcludeSemantics: false(line 406) while child Text widgets are wrapped withExcludeSemantics(lines 414, 429). This creates confusion about what should be announced by screen readers.Non-user-friendly labels: Using
config.key(line 404) as the semantic label exposes technical configuration keys (e.g., "mosip.registration.max.retry") to screen readers instead of human-readable descriptions.Semantic structure: The current structure doesn't provide meaningful context about what each column represents (config key, server value, local value).
Consider restructuring the semantics to provide a coherent, user-friendly description:
return Semantics( label: 'Configuration: ${config.key}. Server value: ${config.serverValue}. Local value: ${config.localValue.isEmpty ? "not set" : config.localValue}', container: true, child: Padding( key: Key('config_row_${config.key}'), padding: const EdgeInsets.symmetric(horizontal: 20, vertical: 15), child: Row( children: [ Expanded( flex: 2, child: Text( config.key, key: Key('config_key_${config.key}'), style: TextStyle( fontSize: 12, fontWeight: config.isModified ? FontWeight.bold : FontWeight.normal, color: config.isModified ? Colors.blue : Colors.black, ), ), ), // ... rest of the row ], ), ), );This provides screen reader users with complete, meaningful information about each configuration row.
500-529: Fix broken Semantics configuration for read-only text.The Semantics wrapper has
excludeSemantics: true(line 506), which prevents the semantic properties (label,hint,tooltipon lines 503-505) from being exposed to screen readers. This makes the entire Semantics wrapper ineffective for accessibility purposes.Additionally, like the editable field, the semantic labels use technical identifiers instead of user-friendly descriptions.
🔧 Proposed fix
Widget _buildReadOnlyText(GlobalConfigItem config) { - final uniqueId = 'local_value_${config.key}'; return Semantics( - label: uniqueId, - hint: uniqueId, - tooltip: uniqueId, - excludeSemantics: true, + label: 'Local value for ${config.key}: ${config.localValue.isEmpty ? "not set" : config.localValue}', + hint: 'This configuration is read-only', + excludeSemantics: false, container: true, child: Container( - key: Key(uniqueId), + key: Key('local_value_${config.key}'), padding: const EdgeInsets.symmetric( horizontal: 8, vertical: 8, ), decoration: BoxDecoration( border: Border.all(color: Colors.grey[300]!), borderRadius: BorderRadius.circular(4), color: Colors.grey[100], ), child: Text( config.localValue.isEmpty ? '-' : config.localValue, style: TextStyle( color: config.isModified ? Colors.blue : Colors.grey[600], fontSize: 12, fontWeight: config.isModified ? FontWeight.bold : FontWeight.normal, ), ), ), ); }
🤖 Fix all issues with AI agents
In @lib/ui/settings/widgets/global_config_settings_tab.dart:
- Around line 459-497: The Semantics wrapper in _buildEditableTextField
currently exposes technical identifiers via uniqueId for label/hint/tooltip;
replace those with user-friendly text derived from the config (e.g., use a
human-readable name like config.displayName or a mapped label for
GlobalConfigItem, and use config.serverValue as the hint/tooltip fallback).
Update the Semantics properties (label, hint, tooltip) and the TextField's
decoration.hintText to use these descriptive strings so screen readers get
meaningful context while falling back to config.key only if no displayName/label
exists.
📜 Review details
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
lib/ui/settings/widgets/global_config_settings_tab.dart
🔇 Additional comments (1)
lib/ui/settings/widgets/global_config_settings_tab.dart (1)
403-529: Verify the accessibility implementation against Flutter's actual API and best practices.The suggestion to use
semanticsLabelon TextField is incorrect—Flutter'sTextFieldwidget does not support asemanticsLabelparameter. The current implementation usingSemantics(textField: true, ...)is actually the recommended approach for providing accessibility labels to text fields.To improve accessibility:
- Consider using a more user-friendly label instead of dev-facing keys (e.g.,
"${config.key} value, editable"instead of'local_value_${config.key}').- Add
labelTexttoInputDecoration(in addition to or instead of relying solely onhintText) to provide clearer field context for screen readers:decoration: InputDecoration( labelText: config.key, // Provides semantic context hintText: config.serverValue, ... )- Consolidate redundant Semantics properties:
hintandtooltipset to the same value aslabelprovide no additional benefit and can confuse assistive technology.Likely an incorrect or invalid review comment.
Signed-off-by: sachin.sp <sachin.sp@cyberpwn.com>
3 participants
RCF-1308
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.