Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Popover] Re-position on content change? #151

Closed
tsneed290 opened this issue Mar 17, 2017 · 12 comments
Closed

[Popover] Re-position on content change? #151

tsneed290 opened this issue Mar 17, 2017 · 12 comments

Comments

@tsneed290
Copy link

Started prototyping in Vue a month ago and stumbled onto this awesome library! Thank you for providing this!

I was messing around with the Popover component and realized that when the content of the popover dynamically changes, the Popover itself does not get re-positioned.

screen shot 2017-03-17 at 11 03 17 am

...then the content changes...

screen shot 2017-03-17 at 11 03 22 am

Mind you I am using this Popover component within a Modal component, might be worth mentioning there.

Also, when the Modal is closed, the Popover component remains, should I be listening to the modal "close" event and hiding the popover manually?

Thanks for any help!
-Tim

@pi0
Copy link
Member

pi0 commented Mar 17, 2017

@tsneed290 Thanks for your feedback. i'll check this and come back to you soon. (Also ping @sirlamer :D he may help us)

@pi0
Copy link
Member

pi0 commented Mar 18, 2017

I can reproduce both bugs now with playground :)

  • beforeDestroy is not invoked in some conditions like route changes or modal show/hides (this problem may be with vue caching)
  • placement is done lazily (when i slightly scroll page tether will re position itself)

UPDATE First problem is tricked by adding more listeners when changing routes or hiding modals.

@pi0 pi0 added the Type: Bug label Mar 18, 2017
pi0 pushed a commit that referenced this issue Mar 18, 2017
@tsneed290
Copy link
Author

tsneed290 commented Mar 20, 2017

Can confirm that simulating a "scroll" event when the popover content changes (via watchers) does fix the position, but it is still lazily changed.

@pi0
Copy link
Member

pi0 commented Mar 20, 2017

@tsneed290 Re-positioning can be easily done pragmatically thanks to Tether API. question is how we can detect that content chenges ? (or worst global layout changes) :)

@tsneed290
Copy link
Author

I'm still a major Vue.js newbie at this point, but couldn't you place a watcher on the "content" property and re-call this.setOptions() like you are with the placement, offset, and constraints props?

@pi0
Copy link
Member

pi0 commented Mar 20, 2017

@tsneed290 Actually it is not possible simply watching content because we are using <slots>. I'll work more on finding out a solution :)

@tsneed290
Copy link
Author

Ah, right :)

Well I was able to call Tether's position() using a Vue $ref to re-position, but it's just as lazy as invoking a "scroll" event.

Also, was not aware that this library had its own CSS file. After I took 0.10.0-rc.1 today I realized my modals stopped working because I was missing the CSS file. Might need to update docs.

@pi0
Copy link
Member

pi0 commented Mar 20, 2017

@tsneed290 Docs are updated but i will add a bolder announcement on README . thanks :)

@tsneed290
Copy link
Author

Thanks @pi0!

Oh, quick update. Calling positon() wasn't lazy. I wasn't waiting for the DOM to update. Using Vue.nextTick() proved to be a good workaround:

  watch: {
    popoverContent () {
      const _tether = this.$refs.valPopover._tether
      Vue.nextTick(function () {
        _tether && _tether.position()
      })
    }
  },

@pi0
Copy link
Member

pi0 commented Mar 20, 2017

Also changes are not limited to content change. (check this ) maybe we can use a dirty workaround : periodically call position() when tooltip is visible. (it may not be SO costly as tooltips are only visible small time)

@GregPeden
Copy link
Contributor

GregPeden commented Mar 21, 2017

Sorry for being distant, I am traveling for work.

The suggestions to hook and resposition on changes are good. I am starting with looking at using MutationObserver to watch the popover element in general, but then might look for a polyfill replacement for continued IE10 support.

My sense is that it will probably be better and more efficient to just say "if you want to have dynamic content, use the 'title' and 'content' properties, and pre-format your HTML". Then put watchers on those.

@pi0
Copy link
Member

pi0 commented Mar 28, 2017

@tsneed290 We have tried hard to make popovers more stable and reacting to content changes. More improvements will come fore sure in future. Closing this for housekeeping re-open please if needed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants