fix: focus issue on translate modal, remove duplicate code #5630
Conversation
luka-nextcloud
force-pushed
the
bugfix/focus-on-translate-modal
branch
from
4bbc4f1 to
77e5121
Compare
|
@max-nextcloud Could you please review again? |
| return commands.insertContentAt(range, content) | ||
| }) | ||
| this.displayTranslate = false | ||
| emit('text:translate-modal:show', { content: this.selection || '' }) |
There was a problem hiding this comment.
Does this even make sense if the selection is empty?
The previous code looks as if it did not open the modal on an empty selection:
this.displayTranslate = this.selectionHowever I just tried it on cloud.nextcloud.com and there it opened anyway but did not work (due to the broken input but also as it had no content).
There was a problem hiding this comment.
Does this even make sense if the selection is empty?
The modal should be open anyway. User is free to update the input text.
If selection is empty, all content will be selected. I updated this behavior to same as action on menu bar.
PR updated:
- select all if selection is empty when open translate dialog
- add cypress test
There was a problem hiding this comment.
Does the $editor.selectAll have some sideeffects? Is the text selected fully afterwards? Otherwise this sounds good.
There was a problem hiding this comment.
@max-nextcloud I didn't find any side effects. Yes, the text is selected fully.
luka-nextcloud
force-pushed
the
bugfix/focus-on-translate-modal
branch
from
77e5121 to
747bfa8
Compare
|
Restarted failing cypress test which seemed unrelated |
Signed-off-by: Luka Trovic <luka@nextcloud.com>
Signed-off-by: Luka Trovic <luka@nextcloud.com>
luka-nextcloud
force-pushed
the
bugfix/focus-on-translate-modal
branch
from
747bfa8 to
9e5b885
Compare
📝 Summary
🏁 Checklist
npm run lint/npm run stylelint/composer run cs:check)