Migrate popovers and tooltips to floating-vue #1868

jovyntls · 2022-04-02T03:12:15Z

What is the purpose of this pull request?

Part of #1702 , by removing the dependency on bootstrap-vue
A copy of #1856

Overview of changes:

Popovers use the Menu component from Floating Vue
Tooltips use the Tooltip component from Floating Vue
Default value of placement has been changed to placement=top to preserve the same behaviour. floating-vue has the option for placement=auto, but this tends to render popups to the right in small containers (where they previously would be on top). Since floating-vue automatically repositions popovers even for non-auto placements, placement=top is the closest to the original behaviour.

Anything you'd like to highlight / discuss:
Should the full removal of bootstrap-vue be done in a separate PR?

Testing instructions:
There should be no noticeable change in behaviour or UI for popovers and tooltips

Proposed commit message: (wrap lines at 72 characters)

Migrate popovers and tooltips to floating-vue

Popovers and tooltips are implemented using bootstrap-vue. 

bootstrap-vue has dependencies on both Vue and Bootstrap, and has not
migrated the latest versions. This is a blocker for MarkBind's
migration.

Let's use floating-vue's Menu and Tooltip components for Popovers and
Tooltips respectively. floating-vue supports both Vue 2 and Vue 3, and
has fewer dependencies. This allows us to migrate to Vue 3 and 
Bootstrap 5 more easily in the future.

Checklist: ☑️

Updated the documentation for feature additions and enhancements
Added tests for bug fixes or features
Linked all related issues
No unrelated changes

ang-zeyu

Some more issues on top of the earlier reviews:

Some tooltips don't seem to be working:
- This one in index.md
- These ones in tooltips.md (oddly, only on my phone)
Let's add a simple fade or scale enter/leave animation similar to what we had for bootstrap-vue as well, so the activation becomes a little less jarring
I'm unable to close this specific tooltip after activating it at times on my phone (desktop seems to be ok). Might be the same issue as the first one.
- Reproduction (a little unreliable, might need a couple of tries):
  - double / spam click the "coupling" word a couple of times
  - try touching some area outside to close the tooltip

Forgot to mention:
Saw that the previous PR was merged as well -- then reverted? 👀
Do ignore these as appropriate and let me know if I'm missing any context!

ryoarmanda · 2022-04-02T11:16:41Z

This one in index.md

Ditto on the above.

These ones in tooltips.md (oddly, only on my phone)

I can access the above on my phone 🤔 not sure what happens here.

I'm unable to close this specific tooltip after activating it at times on my phone (desktop seems to be ok). Might be the same issue as the first one.

Sometimes this happens on my device too. But I found out a sure way to close this, tap on the tooltip content, then tap outside it.

Forgot to mention:

Saw that the previous PR was merged as well -- then reverted? 👀

Few days ago me and Jonah gave the okay for 3281 students to approve and merge PRs. However, the previous PR for this one was merged with a regular merge commit. We decided to undo the merge in order to maintain atomic history, so this PR is created again.

jovyntls · 2022-04-03T03:19:39Z

Let's add a simple fade or scale enter/leave animation similar to what we had for bootstrap-vue as well, so the activation becomes a little less jarring

I think popover/tooltip animations weren't implemented in v3.1.1 either (effect was in the docs but this was an error). Should I still add it to this PR?

This one in index.md

Ditto on the above.

Yep, this doesn't work for me too. Will look into it!

These ones in tooltips.md (oddly, only on my phone)

I can access the above on my phone 🤔 not sure what happens here.

Same for me, it works fine on my phone

I'm unable to close this specific tooltip after activating it at times on my phone (desktop seems to be ok). Might be the same issue as the first one.

Sometimes this happens on my device too. But I found out a sure way to close this, tap on the tooltip content, then tap outside it.

This one works on my phone too (can always be closed by clicking elsewhere).

ang-zeyu · 2022-04-03T06:42:57Z

I think popover/tooltip animations weren't implemented in v3.1.1 either (effect was in the docs but this was an error). Should I still add it to this PR?

Hmm not sure if we're looking at the same link https://markbind.org/userGuide/components/popups.html#tooltips
(not so much the presence of the effect prop, the animation is built into bootstrap-vue if I'm not mistaken, try hovering over it)

Yep, this doesn't work for me too. Will look into it!

This seems fixed for me now

Same for me, it works fine on my phone

Figured out a workaround -- holding down on it seems to work 👀

But I found out a sure way to close this, tap on the tooltip content, then tap outside it.

It should be fixed of course (we can't expect the reader to know the holding down or above trick), hopefully both behaviours boil down to the same issue below.

This one works on my phone too (can always be closed by clicking elsewhere).

I'm able to reproduce this very reliably now with the newest commit (previously, double/spam clicking seems to do it more reliably).
Just noticed it also occurs with the "tooltip on top/etc" buttons. (I think all the hover ones)

packages/vue-components/src/Popover.vue

jovyntls · 2022-04-03T06:47:37Z

Thanks for testing @ryoarmanda @ang-zeyu ! Sorry for the force-pushing, I was trying out a new fix by handling <input>s separately, which allows for more specific handling of triggers.

Hmm not sure if we're looking at the same link markbind.org/userGuide/components/popups.html#tooltips
(not so much the presence of the effect prop, the animation is built into bootstrap-vue if I'm not mistaken, try hovering over it)

It doesn't have the animation for me:

Screen.Recording.2022-04-03.at.2.44.46.PM.mov

ang-zeyu · 2022-04-03T06:50:52Z

It doesn't have the animation for me:

Hmm works for me (chrome, windows). Might be a browser issue with chrome on mac; See if you can get it working on something else (firefox / safari).

jovyntls · 2022-04-03T06:54:46Z

Hmm works for me (chrome, windows). Might be a browser issue with chrome on mac; See if you can get it working on something else (firefox / safari).

No animations on:

Chrome (Mac)
Safari (Mac/iOS)
Firefox (Mac)

No animations on the bootstrap-vue website as well.

EDIT: some screen recordings
Firefox:

Screen.Recording.2022-04-03.at.2.55.36.PM.mov

Safari:

Screen.Recording.2022-04-03.at.2.56.28.PM.mov

ang-zeyu · 2022-04-03T07:02:54Z

👀👀 Tried a couple of other browsers and devices (could try this), still ok on my end

There is an https://developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-reduced-motion accessibility media query that could interfere as well (check if you have reduce motion)

jovyntls · 2022-04-03T07:08:32Z

👀👀 Tried a couple of other browsers and devices (could try this), still ok on my end

There is an developer.mozilla.org/en-US/docs/Web/CSS/@media/prefers-reduced-motion accessibility media query that could interfere as well (check if you have reduce motion)

You're right, turning off my reduced motion settings brought the fade back! 😭 Will add it back to the pop-ups implementations. Thanks for helping to investigate @ang-zeyu !

ang-zeyu · 2022-04-03T07:40:23Z

packages/vue-components/src/Popover.vue

@@ -21,7 +21,6 @@
      :hide-triggers="triggers"
      :placement="placement"
      :delay="0"
-      popper-class="v-popper__popper--skip-transition"


👀 This does the trick for the previous issues, seems like everything was related.

(just speculation: perhaps v-popper__popper--skip-transition isn't meant to be used externally? -- not sure)

Think we could @ryoarmanda to do another check too, since a lot of the issues were device specific.

👀 This does the trick for the previous issues, seems like everything was related.

(just speculation: perhaps v-popper__popper--skip-transition isn't meant to be used externally? -- not sure)

Hmm I believe so! floating-vue's docs mention v-popper__popper--skip-transition as a possible helper class though it seems like it's best used in conjunction with floating-vue's CSS themes as a dynamic class.

The hover issues were fixed in 55a888—had to separate the handling of <input>s because wrapping <input> in a wrapper element results in floating-vue being unable to detect its focus events, but failing to wrap other tags in a wrapper element prevents floating-vue from detecting mouse events (which was what caused the un-openable tooltip in index.md).

The behaviours are now fixed on my end 👍

ang-zeyu

Lgtm 👍

ryoarmanda

👍 The issues seem to be resolved now, looks good to me!

ang-zeyu

One more error, dosen't look like a hydration issue:

jovyntls · 2022-04-03T15:04:17Z

One more error, dosen't look like a hydration issue:

Thanks for the catch! It doesn't appear for me when serving locally, so I'll push some changes and check if it fixes it; will update this message when it's done

edit: Fixed!

commit 32f2e64 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Mon Apr 18 01:11:00 2022 +0800 Fix typos in docs (MarkBind#1902) commit cac6027 Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Sun Apr 17 22:34:15 2022 +0800 Remove package-lock file in packages/core (MarkBind#1908) commit 6a7ea88 Merge: 30e3637 8106d48 Author: Jonah Tan <47470981+jonahtanjz@users.noreply.github.com> Date: Fri Apr 15 23:52:47 2022 +0800 Migrate Bootstrap to v5.1.3 (MarkBind#1882) commit 8106d48 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Fri Apr 15 23:31:54 2022 +0800 Prevent Navbars from being printed (MarkBind#1906) commit 30e3637 Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Fri Apr 15 18:40:55 2022 +0800 Update CI steps to set up Graphviz and Java for PlantUML support (MarkBind#1905) PlantUML feature requires both Java and Graphviz to be installed. Current CI steps do not guarantee the presence of these dependencies. This means some PlantUML diagrams were unable to be generated correctly. Let's update our CI steps to ensure both Java and Graphviz are available in the build image to support PlantUML syntax. This will ensure usage of PlantUML syntax can be supported in our tests and our documentation. commit d7c6819 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Fri Apr 15 12:35:54 2022 +0800 Migrate Bootswatch themes to v5.1.3 (MarkBind#1898) commit aadc9b6 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Wed Apr 13 16:25:34 2022 +0800 Refactor includePanelProcessor and MdAttributeRenderer (MarkBind#1896) commit 33383ca Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Wed Apr 13 01:11:23 2022 +0800 Migrate horizontal rules (MarkBind#1901) In Bootstrap 5, `<hr>` elements now use height instead of border to better support the size attribute. However, since MarkBind's hr styles support dotted/dashed/double lines, `<hr>` needs a border-based implementation. Let's minimally override Bootstrap 5's `<hr>` styling to continue supporting styled horizontal rules. commit eef4089 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Wed Apr 13 01:10:58 2022 +0800 Fix spacing for navbars and searchbars (MarkBind#1900) commit 84099ac Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Tue Apr 12 15:05:17 2022 +0800 Migrate MarkBind's quizzes and questions (MarkBind#1890) commit 5485206 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Tue Apr 12 13:23:14 2022 +0800 Namespace Bootstrap data attributes with data-bs- (MarkBind#1892) commit 0ad144d Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Tue Apr 12 11:57:14 2022 +0800 Migrate dropdown components (MarkBind#1888) commit 5f9cc95 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Tue Apr 12 00:02:32 2022 +0800 Remove jumbotron class (MarkBind#1885) commit c1436a1 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Sat Apr 9 11:53:07 2022 +0800 Migrate close buttons (MarkBind#1886) commit 7bc7878 Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Thu Apr 7 15:46:51 2022 +0800 Migrate breaking changes for badges (MarkBind#1884) commit e35c32b Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Tue Apr 5 21:15:24 2022 +0800 Migrate utilities to Bootstrap V5 (MarkBind#1834) commit 9d1dcdd Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Tue Apr 12 11:56:53 2022 +0800 Update GitHub issue template checkbox to dropdown (MarkBind#1897) commit 950b422 Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Sat Apr 9 18:15:17 2022 +0800 Fix UG display issues when using the outputBox boilerplate (MarkBind#1889) commit 7608553 Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Thu Apr 7 15:10:37 2022 +0800 Add an explanation on how to use JavaScript chart libraries (MarkBind#1872) Charts can be useful in MarkBind sites to display data. Let's include a section in our UG to highlight the existing (basic) support for JavaScript chart libraries in MarkBind. This is the most flexible option that MarkBind users can go for to utilize a JavaScript chart library of their choice. commit f14ea43 Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Wed Apr 6 01:59:22 2022 +0800 Fix edge cases of link validation and conversion (MarkBind#1875) Invalid intralinks have been reported in MarkBind docs. The reasons are 1. actual invalid link 2. file path contains a space 3. Email link wrongly validated. Let's fix all three link validation & conversion errors. File paths with %20 will be correctly checked for existence. Email(tel) links will no longer be appended with the base URL or validated as intralink. The removal of invalid link behavior will allow the link & link validation mechanism to function properly. commit 4f060c6 Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Tue Apr 5 21:50:39 2022 +0800 Update CI to run on all PR events (MarkBind#1878) commit 7673e66 Author: Liu YongLiang <41845017+tlylt@users.noreply.github.com> Date: Tue Apr 5 21:44:19 2022 +0800 Update documentation on MarkBind GitHub Actions (MarkBind#1880) The existing GitHub Action "markbind-action" has been revamped to improve its functionalities. Let's add a reference to "markbind-action" in our user guide to highlight this helper workflow. Let's also include a section in our developer guide to explain how to develop and maintain the "markbind-action" repo. This helps users discover "markbind-action" and use it in their CI/CD workflow. The dev docs will help developers continue to improve "markbind-action" commit feee60a Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Tue Apr 5 11:19:51 2022 +0800 Remove bootstrap-vue (MarkBind#1873) bootstrap-vue is no longer used in MarkBind as it relies on Vue 2 and Bootstrap 4, both which we plan to upgrade. Its components have been migrated to other libraries. Let's remove all mentions and instances of bootstrap-vue from MarkBind to fully remove this dependency. commit 1bd81ff Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Mon Apr 4 13:35:06 2022 +0800 Add Components page to developer guide (MarkBind#1865) Add Writing Components page to dev docs commit dac728d Author: Jovyn Tan <61113575+jovyntls@users.noreply.github.com> Date: Mon Apr 4 12:00:48 2022 +0800 Migrate popovers and tooltips to floating-vue (MarkBind#1868) Popovers and tooltips are implemented using bootstrap-vue. bootstrap-vue has dependencies on both Vue and Bootstrap, and has not migrated the latest versions. This is a blocker for MarkBind's migration of these dependencies. Let's use floating-vue's Menu and Tooltip components for Popovers and Tooltips respectively. floating-vue supports both Vue 2 and Vue 3, and has fewer dependencies. This allows us to migrate to Vue 3 and Bootstrap 5 more easily in the future.

jovyntls added 29 commits March 27, 2022 14:30

minimal test

fdbe390

Working modal

7508691

Fix overflow

4107622

Fix SSR and add zoom effect

6d4b8db

Remove accidental files

d5ab037

Minor cleanup

2d272e5

Restore deprecated modal-header and modal-footer

ccc423e

Fix tests and deprecated slots

e04d4aa

Add tests

938b56a

Upgrade Vue to v2.6.14

6243e26

Update vue-final-modal to v2.4.1

c209f0d

minimal

87e0092

Working popovers tooltips triggers

70e625a

Replicate existing behaviour

d9f1be3

Replicate styles

82cff18

Fix package-lock

7ac6715

Add snapshot tests

fa1eae1

Update vue-components README

f095cb5

Fix package-lock.json

cd596ec

Update package-lock.json

ed411d4

Fix merge mistakes

6622078

Fix merge mistakes again

696d529

Remove extra eslint disable

46671fb

Update tests

bd35db7

Fix test

589b1cf

Fix missing modal triggers

8b6415b

Replicate existing tooltips

e166c57

Linting

3b86f5b

Update snapshot tests

6500e0d

ang-zeyu reviewed Apr 2, 2022

View reviewed changes

jovyntls force-pushed the floating-ui-popups branch from 679c645 to 5c8b59e Compare April 3, 2022 05:10

jovyntls added 2 commits April 3, 2022 14:39

Handle input separately

55a8889

Update tests

b5fc282

jovyntls force-pushed the floating-ui-popups branch from 6e1fd3f to b5fc282 Compare April 3, 2022 06:42

ang-zeyu reviewed Apr 3, 2022

View reviewed changes

packages/vue-components/src/Popover.vue Show resolved Hide resolved

jovyntls added 2 commits April 3, 2022 15:09

Re-enable fade animations

94e87bf

Update tests

9b2b78a

ang-zeyu reviewed Apr 3, 2022

View reviewed changes

ang-zeyu approved these changes Apr 3, 2022

View reviewed changes

ryoarmanda approved these changes Apr 3, 2022

View reviewed changes

ryoarmanda added this to the 4.0 milestone Apr 3, 2022

ang-zeyu reviewed Apr 3, 2022

View reviewed changes

Add check for undefined

059086f

ang-zeyu merged commit dac728d into MarkBind:master Apr 4, 2022

jovyntls mentioned this pull request Apr 4, 2022

Remove bootstrap-vue #1873

Merged

9 tasks

jovyntls mentioned this pull request Apr 15, 2022

Migrate to Bootstrap 5 #1882

Merged

51 tasks

kaixin-hc mentioned this pull request Feb 14, 2024

Tooltips seem to add an extra space in front #2419

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Migrate popovers and tooltips to floating-vue #1868

Migrate popovers and tooltips to floating-vue #1868

jovyntls commented Apr 2, 2022

ang-zeyu left a comment •

edited

Loading

ryoarmanda commented Apr 2, 2022 •

edited

Loading

jovyntls commented Apr 3, 2022 •

edited

Loading

ang-zeyu commented Apr 3, 2022

jovyntls commented Apr 3, 2022 •

edited

Loading

ang-zeyu commented Apr 3, 2022

jovyntls commented Apr 3, 2022 •

edited

Loading

ang-zeyu commented Apr 3, 2022

jovyntls commented Apr 3, 2022

ang-zeyu Apr 3, 2022

ang-zeyu Apr 3, 2022

jovyntls Apr 3, 2022

ryoarmanda Apr 3, 2022

ang-zeyu left a comment

ryoarmanda left a comment

ang-zeyu left a comment

jovyntls commented Apr 3, 2022 •

edited

Loading

Migrate popovers and tooltips to floating-vue #1868

Migrate popovers and tooltips to floating-vue #1868

Conversation

jovyntls commented Apr 2, 2022

ang-zeyu left a comment • edited Loading

Choose a reason for hiding this comment

ryoarmanda commented Apr 2, 2022 • edited Loading

jovyntls commented Apr 3, 2022 • edited Loading

ang-zeyu commented Apr 3, 2022

jovyntls commented Apr 3, 2022 • edited Loading

ang-zeyu commented Apr 3, 2022

jovyntls commented Apr 3, 2022 • edited Loading

ang-zeyu commented Apr 3, 2022

jovyntls commented Apr 3, 2022

ang-zeyu Apr 3, 2022

Choose a reason for hiding this comment

ang-zeyu Apr 3, 2022

Choose a reason for hiding this comment

jovyntls Apr 3, 2022

Choose a reason for hiding this comment

ryoarmanda Apr 3, 2022

Choose a reason for hiding this comment

ang-zeyu left a comment

Choose a reason for hiding this comment

ryoarmanda left a comment

Choose a reason for hiding this comment

ang-zeyu left a comment

Choose a reason for hiding this comment

jovyntls commented Apr 3, 2022 • edited Loading

ang-zeyu left a comment •

edited

Loading

ryoarmanda commented Apr 2, 2022 •

edited

Loading

jovyntls commented Apr 3, 2022 •

edited

Loading

jovyntls commented Apr 3, 2022 •

edited

Loading

jovyntls commented Apr 3, 2022 •

edited

Loading

jovyntls commented Apr 3, 2022 •

edited

Loading