Fix home page button interaction blocked by MergeActionBar #3943
Conversation
Wrap MergeActionBar with IgnorePointer to prevent it from intercepting touch events when not in selection mode. This fixes the issue where bottom navigation buttons, record button, and chat button were unresponsive on the home page.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request correctly fixes an issue where the MergeActionBar was blocking touch events on the home page by wrapping it with an IgnorePointer. This is a good solution to the problem.
However, the changes also introduce a critical regression by removing internationalization support from this widget and replacing localized strings with hardcoded English text. This will prevent the UI from being displayed in other languages and makes future localization efforts more difficult. I have left a comment with a suggestion to restore the use of the localization framework. Please address this before merging.
| child: const Text( | ||
| 'Cancel', | ||
| style: TextStyle( | ||
| color: Color(0xFF8E8E93), | ||
| fontSize: 17, | ||
| fontWeight: FontWeight.w500, | ||
| ), | ||
| ), |
There was a problem hiding this comment.
This pull request introduces a significant regression by removing internationalization support and hardcoding user-facing strings in English. This makes the application difficult to localize and maintain. All user-facing text should be retrieved from localization files using context.l10n.
This issue affects multiple strings in this file:
- 'Cancel' (line 85)
- '$count selected' (line 101)
- 'Merge' (line 133)
- 'Merging in background. This may take a moment.' (line 172)
- 'Failed to start merge' (line 191)
Please revert these changes to use the localization framework. You will also need to re-add the import 'package:omi/utils/l10n_extensions.dart'; at the top of the file.
| child: const Text( | |
| 'Cancel', | |
| style: TextStyle( | |
| color: Color(0xFF8E8E93), | |
| fontSize: 17, | |
| fontWeight: FontWeight.w500, | |
| ), | |
| ), | |
| child: Text( | |
| context.l10n.cancel, | |
| style: const TextStyle( | |
| color: Color(0xFF8E8E93), | |
| fontSize: 17, | |
| fontWeight: FontWeight.w500, | |
| ), | |
| ), |
|
@adithya-234 can you pls share a before and after video? |
|
@mdmohsin7 sure will send |
2 participants
Summary
Fixes home page button interaction being blocked by the MergeActionBar component.
Problem: Bottom navigation buttons, record button, and chat button were unresponsive on the home page because the MergeActionBar was intercepting touch events even when not in selection mode.
Solution: Wrapped the MergeActionBar with IgnorePointer to prevent it from intercepting touch events when not in selection mode, allowing touches to pass through to underlying components.
Changes
Test Plan