feat: enable direction for card title, group column header #5429
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Hi, @zoli. The PR has been merged. |
* main: (52 commits) chore: improve popup menu color (AppFlowy-IO#5725) fix: ai chat result contains html escape text (AppFlowy-IO#5721) chore: bump version 0.6.4 (AppFlowy-IO#5719) feat: support sign-in and sign-up on Web (AppFlowy-IO#5712) fix: syntax error when generating translation files (AppFlowy-IO#5718) feat: choose cursor/selection color from palette or color picker AppFlowy-IO#5041 (AppFlowy-IO#5677) feat: render ai text message with appflowy_editor (AppFlowy-IO#5682) feat: enable debug logs in internal build (AppFlowy-IO#5713) chore: update yrs and appflowy collab dependencies (AppFlowy-IO#5707) chore: bump version 0.6.3 (AppFlowy-IO#5701) feat: support publish document (AppFlowy-IO#5576) refactor: extract chat plugin to new repo (AppFlowy-IO#5699) fix: take the max value of the keyboard height and the view insets bottom to make the toolbar visible (AppFlowy-IO#5700) chore: use latest macos runner to build the release package (AppFlowy-IO#5686) fix: filter chat page when duplicating (AppFlowy-IO#5676) chore: show ai service error (AppFlowy-IO#5675) feat: sync the created view after duplicating (AppFlowy-IO#5674) chore: disable cloud search (AppFlowy-IO#5663) feat: support moving page to a space (AppFlowy-IO#5665) chore: update German translations (AppFlowy-IO#5640) ...
* main: fix: integration test failed (grid row detail page: hide and show hidden fields) (AppFlowy-IO#5781) feat: ai billing (AppFlowy-IO#5741) fix: can not display rows when rows overthan 1000 (AppFlowy-IO#5777) feat: support publish database on web (AppFlowy-IO#5748) feat: publish databse to Web (AppFlowy-IO#5709) chore: update Spanish translations (AppFlowy-IO#5742) chore: update Chinese translations (AppFlowy-IO#5727) fix: Add retry for admin client sign in for test (AppFlowy-IO#5767) chore: update Hebrew translation (AppFlowy-IO#5738) chore: update German translations (AppFlowy-IO#5722) chore: update Russian translations (AppFlowy-IO#5730) chore: update build config (AppFlowy-IO#5759) chore: enable local ai and local ai chat (AppFlowy-IO#5755) chore: bump version 0.6.4 (AppFlowy-IO#5744) fix: improve color selector (AppFlowy-IO#5743) fix: reset space relationship when clearing cache (AppFlowy-IO#5737) chore: show plugin state (AppFlowy-IO#5740) chore: download llm files (AppFlowy-IO#5723) feat: optimize the read recent views speed (AppFlowy-IO#5726) chore: fix compile (AppFlowy-IO#5733)
LucasXu0
reviewed
Oct 16, 2024
| import 'package:appflowy/workspace/application/settings/appearance/appearance_cubit.dart'; | ||
| import 'package:appflowy_editor/appflowy_editor.dart'; | ||
|
|
||
| TextDirection getTextDirectionBaseOnContext( |
There was a problem hiding this comment.
I would suggest adding these functions to the BuildContext extension to allow easy accessibility.
| emoji: state.rowMeta.icon, | ||
| showEmojiPicker: () => popoverController.show(), | ||
| Widget? emojiWidget, child; | ||
| if (state.rowMeta.icon.isNotEmpty) { |
There was a problem hiding this comment.
The widget builder logic is not clear here. It's not good for maintenance.
| return BlocBuilder<TextCellBloc, TextCellState>( | ||
| builder: (context, state) { | ||
| final text = _textEditingController.text.isNotEmpty |
There was a problem hiding this comment.
How about setting LocaleKeys.grid_row_titlePlaceholder.tr() as the hint text instead of the text?
| color: Theme.of(context).colorScheme.primary, | ||
| return ValueListenableBuilder( | ||
| valueListenable: _controller, | ||
| builder: (context, TextEditingValue value, __) { |
There was a problem hiding this comment.
Using _ (single underscore).
Comment on lines
291
to
299
| class _TitleSkin extends IEditableTextCellSkin { | ||
| _TitleSkin({this.emojiWidget}); | ||
|
|
||
| final Widget? emojiWidget; | ||
| ui.TextDirection? lastDirection; | ||
|
|
||
| @override | ||
| Widget build( | ||
| BuildContext context, |
There was a problem hiding this comment.
I'm not sure I like this.
Auto bidi is okay for text fields, but if we want to reverse the positioning of widgets in a row, the user should choose RTL from the settings. Otherwise, there are too many changes that we need to do throughout the entire application.
Comment on lines
+314
to
+323
| final cText = c.content.isNotEmpty | ||
| ? c.content | ||
| : LocaleKeys.grid_row_titlePlaceholder.tr(); | ||
| final cTextDirection = getTextDirectionBaseOnContext( | ||
| context, | ||
| cText, | ||
| lastDirection: lastDirection, | ||
| ); | ||
|
|
||
| return lastDirection != cTextDirection; |
There was a problem hiding this comment.
We can't rely on the text in the bloc state. We might be only saving the text upon submission, so this won't react quickly enough. A better direction will be listening to the text editing controller like you've done in other places
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
resolves #5387
This depends on AppFlowy-IO/appflowy-editor#809.
Feature Preview
appflowy-board-auto-direction.webm
PR Checklist