Skip to content
This repository has been archived by the owner on Oct 9, 2024. It is now read-only.

Bug/make chec-options-menu popout #83

Merged
merged 4 commits into from
Mar 18, 2020

Conversation

john-raymon
Copy link
Contributor

@john-raymon john-raymon commented Mar 17, 2020

  • temporarily solves Table overflow prevents popovers showing over them #81
  • replaces relative with static position on root ChecOptionsMenu element to allow popping out of BasePopover, removes need of left position, makes it default positioning without needing to specify left. (not sure if should also remove the require method, using the include, now it only accepts one value)

https://deploy-preview-83--angry-joliot-6c09db.netlify.com/?path=/story/components-options-menu--default-story

image

@john-raymon
Copy link
Contributor Author

john-raymon commented Mar 17, 2020

oops let me fix the commits

edit: fixed

@john-raymon john-raymon force-pushed the feature/make-chec-options-menu-popout branch from 2f57106 to 18aa413 Compare March 17, 2020 00:50
@ScopeyNZ
Copy link
Contributor

I don't know if this is an existing bug, but if the menu is on the right side of the page, it overflows to the right. If that's an existing bug I'm okay merging this.

I really think the "real" solution here is to use something like popper, which takes care of all of these annoying edge cases for us.

@john-raymon
Copy link
Contributor Author

I don't know if this is an existing bug, but if the menu is on the right side of the page, it overflows to the right. If that's an existing bug I'm okay merging this.

I really think the "real" solution here is to use something like popper, which takes care of all of these annoying edge cases for us.

Oooh right I didn't think of that. Let me look into it now and implement it within this
PR.

@john-raymon
Copy link
Contributor Author

john-raymon commented Mar 17, 2020

@robbieaverill @ScopeyNZ setting up vue-popperjs, despite it using deprecated slot= syntax, see RobinCK/vue-popper#88. Maybe we use a fork of ours with that issue resolved, seems easy.

Either way, works with the deprecated slot syntax name

@john-raymon
Copy link
Contributor Author

john-raymon commented Mar 17, 2020

implemented vue-popperjs, using forked version accepting showPopperDefault prop initialize showPopper state of vue-popperjs . (https://github.com/john-raymon/vue-popper/blob/76cc42c971f799cbd5c717b5bb33ae90a9ab61ea/src/component/popper.js.vue#L266)

image

@john-raymon
Copy link
Contributor Author

john-raymon commented Mar 17, 2020

added shadow-md to BasePopover to match Figma designs.

image

@john-raymon john-raymon requested a review from a user March 17, 2020 04:40
Copy link
Contributor

@robbieaverill robbieaverill left a comment

Choose a reason for hiding this comment

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

The menus don't have alignment any more - for example, there's a "right aligned" story for the options menu, but all of the menus show up below the toggle, evenly spaced either side of it.

Otherwise, I would suggest being careful using libraries with low popularity - they're likely to not be well maintained (for example, this library hasn't had any commits this year, and the issue you linked hasn't has a maintainer respond to it).

Was using popper.js directly not an option?

@john-raymon john-raymon force-pushed the feature/make-chec-options-menu-popout branch 2 times, most recently from 272b21a to 33d4afe Compare March 18, 2020 07:11
@john-raymon
Copy link
Contributor Author

The menus don't have alignment any more - for example, there's a "right aligned" story for the options menu, but all of the menus show up below the toggle, evenly spaced either side of it.

Otherwise, I would suggest being careful using libraries with low popularity - they're likely to not be well maintained (for example, this library hasn't had any commits this year, and the issue you linked hasn't has a maintainer respond to it).

Was using popper.js directly not an option?

Temporarily not center aligning below the button, but it's working with popper.js now respecting overflow and flipping too if necessary.

@john-raymon john-raymon changed the title Feature/make chec-options-menu popout Bug/make chec-options-menu popout Mar 18, 2020
.vscode/settings.json Outdated Show resolved Hide resolved
src/components/ChecOptionsMenu.vue Outdated Show resolved Hide resolved
src/components/ChecOptionsMenu.vue Outdated Show resolved Hide resolved
@john-raymon john-raymon force-pushed the feature/make-chec-options-menu-popout branch from 9eafde9 to fffc35a Compare March 18, 2020 21:48
…of basepopover, remove redundant code expecting left position prop
*using forked version accepting
showPopperDefault prop
initialize showPopper state of vue-popperjs  <popper>
to match Figma designs
@john-raymon john-raymon force-pushed the feature/make-chec-options-menu-popout branch from fffc35a to 4c2ee4b Compare March 18, 2020 21:52
@robbieaverill robbieaverill merged commit e43781c into master Mar 18, 2020
@robbieaverill robbieaverill deleted the feature/make-chec-options-menu-popout branch March 18, 2020 23:46
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants