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] [tooltip] mixin #527

Merged
merged 23 commits into from Jun 20, 2017
Merged

[popover] [tooltip] mixin #527

merged 23 commits into from Jun 20, 2017

Conversation

tmorehouse
Copy link
Member

Moved common functionality + tether into popover.js mixin.

This will help auto documentation so that only applicable props are shown in each component's docs.

Currently b-tooltip docs list a lot of props that are only applicable to b-popover, so this will help prevent some usage confusion such as issue #467 (#467 (comment))

This PR replaces outdated PR #470, so that it incorporates changes from PR #485 which addressed issue #484

Coerce slide nodelist into an Array
Copy link
Member

@alexsasharegan alexsasharegan left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like it's just a solid move to mixin vs. extends. 👍

@tmorehouse
Copy link
Member Author

Yep... using a mixin will hep with the auto-documentation generation of supported props on both tooltip and popover.

@alexsasharegan
Copy link
Member

It looks like you also try to structure the order of template props (I've been on a mission to make my own ordering consistent). I wonder if there is a style guide for this yet. I've been working in this order more or less:

  • v-directive
  • id, class, other std html attrs (with bound attrs coming second)
  • event handlers

I imagine that's something the Vue community will eventually settle on. That and little things like closing the opening markup tag on the same line or a new line.

@tmorehouse
Copy link
Member Author

Yeah, I find having the events last makes things easier to see, and bound stuff near the first. The closing markup tag on a newline helps show the end of attributes/bindings/events as sometimes they line up with the next child element

@alexsasharegan
Copy link
Member

We could probably use a standard set of Vetur formatting settings to keep things consistent. What editor do you use when working with Vue code?

@tmorehouse
Copy link
Member Author

I'm old school and use either the github web interface, or or larger changes I edit in notepad++ :P

@alexsasharegan
Copy link
Member

image

@tmorehouse
Copy link
Member Author

tmorehouse commented Jun 19, 2017

I use vim as well ;-) (and vi if no vim)

@alexsasharegan
Copy link
Member

I've managed to get as far as learning how to quit vim—and I feel accomplished there!

@tmorehouse
Copy link
Member Author

Any objections to this PR being merged?

@alexsasharegan
Copy link
Member

None from this guy. Onward!

@tmorehouse tmorehouse merged commit 6f759f4 into bootstrap-vue:master Jun 20, 2017
@tmorehouse tmorehouse added this to Done in Roadmap Jun 23, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

2 participants