feat(component): added fw-popover component #231
Conversation
The component fw-popover can be used whever an element need to be shown elevated like dropdown or tooltip. It will automatically takes care of postion, clipping, etc.,
| @@ -0,0 +1,97 @@ | |||
| import { Component, Element, State, Method, Prop, h } from '@stencil/core'; | |||
| import { createPopper, Instance } from '@popperjs/core'; | |||
There was a problem hiding this comment.
Shall we use the lite import with specific modifiers!
import { createPopper } from '@popperjs/core/lib/popper-lite';
There was a problem hiding this comment.
We are consuming the modules flip and preventOverflow which are not available in the lite version. Lite + flip + preventOverflow modules is almost equals the popperjs core lib.
There was a problem hiding this comment.
Ok then if that is the case... it should be fine!
| @@ -0,0 +1,9 @@ | |||
| .popper { | |||
There was a problem hiding this comment.
Why not use the :host directly as the class?
In the render template, you can directly use the <Host> tag instead of the div making the markup more cleaner!
There was a problem hiding this comment.
Thanks, for the suggestion. Will implement it via the :host selector.
| .popper { | ||
| width: 100%; | ||
| display: none; | ||
| z-index: 99; |
There was a problem hiding this comment.
Do we need to specify z-index or will popper.js handle the clipping part?
| @Method() | ||
| async show() { | ||
| if (!this.isOpen) { | ||
| this.popperInstance = createPopper( |
There was a problem hiding this comment.
Based on this logic what is the purpose of this.isOpen?
Can you give an example of how the consuming component will wire this up?
There was a problem hiding this comment.
The purpose of this.isOpen flag is to prevent the re-render of the component if it's already created. Since this.popperInstance is a state variable changing it will trigger the re-render.
The consuming component's like tooltip or select will use the the element reference and call the show or hide method - during focus-in and focus-out events in case of tooltip and click events in case of select.
There was a problem hiding this comment.
This looks like a very cumbersome way to wire up the component... where the parent component will be doing most of the heavy lifting by making sure it needs to call the.
it may not just be the focus-in & focus-out states...Can you wire this up with the select component to see it in action!? I would like to see if a further abstraction can be moved in the popover component!
If popperInstance is a reference to the created popper object, why do we need it to be a State attribute?
Finally, let's not mix tooltips requirements into this component. Since we have named this component as popover... I feel it would make sense if we keep this only for Dropdown, Select, Calendar picker etc. which has a rounded radius, with shadow and background, with a display animation.
For tooltip, we can create a similar implementation if required or break it into a private class that can be extended for both.
renamed methods for fw-option-list and moved hardcoded strings into props.
renamed renderSelectOption to renderSelectOptions
feat: added fw-list-options component and wiring up fw-popover to fw-select
Checklist:
How Has This Been Tested?