Fix toolbar buttons after preview
What does this MR do and why?
Fix toolbar buttons after preview
Changelog: fixed
Further details
Fixing #416453 (closed) using !125366 (merged) introduced a regression ( #417443 (closed)). This MR reverts the change in !125366 (merged), while keeping its regression spec, and also adding a regression test for #417443 (closed), thereby verifying that both #416453 (closed) and #417443 (closed) are now fixed, and if they break again will be caught in the pipeline.
This MR changes the v-if
introduced in !120850 (diffs) to v-show
. Mixing Vue and jQuery code is a bad practice and causes some unintended side effects if v-if
is used since the component is destroyed and mounted again. However we cannot have v-show
on a <template>
tag, so the only solution was to add v-show
on all the <toolbar-button>
elements in header.vue
.
Wrapping the elements in a div
isn't a valid solution since that would then break the flex layout with the preview button. See #416453 (comment 1456513072) for further details on my investigation on why this is currently the best way to fix this problem.
A better fix would be to remove our reliance on jQuery code, but that is a larger maintenancerefactor effort we need to properly estimate and plan for.
Screenshots or screen recordings
Screenshots are required for UI changes, and strongly recommended for all other merge requests.
Before | After |
---|---|
How to set up and validate locally
- Add a comment, click Switch to plain text editing
- Click Preview
- Click Continue editing
- Select some text
- Click Bold icon in the toolbar.
- Notice the bold action should apply just once.
You can follow similar steps for all other actions in the toolbar.
MR acceptance checklist
This checklist encourages us to confirm any changes have been analyzed to reduce risks in quality, performance, reliability, security, and maintainability.
-
I have evaluated the MR acceptance checklist for this MR.
Related to #417443 (closed)