Skip to content

Fix toolbar buttons after preview

Himanshu Kapoor requested to merge 417443-preview-fix into master

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
image.png image.png

How to set up and validate locally

  1. Add a comment, click Switch to plain text editing
  2. Click Preview
  3. Click Continue editing
  4. Select some text
  5. Click Bold icon in the toolbar.
  6. 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.

Related to #417443 (closed)

Edited by Himanshu Kapoor

Merge request reports

Loading