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

Fix share popover #793

merged 1 commit into from Feb 21, 2017


None yet
2 participants

noirbizarre commented Feb 21, 2017

Currently the share popover is broken on the production build:

screenshot-next data gouv fr 2017-02-20 23-17-13

This PR refactor a little bit the popover transclusion to work in production mode.

This current implementation was relying on Vue.js debug symbols (ie. _vue_directives attribute on elements).

The gain is double:

  • one directive deleted
  • now rely on native querySelector

@noirbizarre noirbizarre added the bug label Feb 21, 2017

@noirbizarre noirbizarre added this to the 1.0.3 milestone Feb 21, 2017

@@ -93,6 +93,12 @@ export function install(Vue) {
}, popover));
// Transclude child content if found
const content = this.el.querySelector('[popover-content]');

This comment has been minimized.


davidbgk Feb 21, 2017


Why not using a class or a data- attribute?

This comment has been minimized.


noirbizarre Feb 21, 2017


because I simply removed the v- prefix and used an attribute.
I can move it to a data-attribute if you prefer

This comment has been minimized.


davidbgk Feb 21, 2017


As you wish, consistency is more important in that case and I don't know what is the current best practice for others vuejs files.

@noirbizarre noirbizarre merged commit 217bf11 into opendatateam:master Feb 21, 2017

1 check passed

ci/circleci Your tests passed on CircleCI!

@noirbizarre noirbizarre deleted the noirbizarre:fix-share-popover branch Feb 21, 2017

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