europa-component-library
europa-component-library copied to clipboard
feat(popover): add component - FRONT-3698
- svg illustrations missing for the component website pages
🚀 Deployed on https://ecl-preview-2615--europa-component-library.netlify.app
- i'm not sure about the requirements regarding the positioning of the popover, but we currently only support bottom and top positioning, with an additional horizontal "offset".
- It is not clear the scope of this component, if this is just a "wrapper" for a list, it would make it look like a variant of the list itself more than an independent component. If this is instead meant to hold different content, (we set a content block...) the lack of support for an horizontal displacement of the popover seems a limitation
- Still about the scope of the component, we currently always and only trigger the content through a link, which makes me think that the "popover" component should not define the trigger, which could be a link, a button that we already have as base components, so the popover could and probably should only define the "content" and the functionality leaving the trigger to an implementation of the link and the button, with an include of the popover template if sets as such.
- There is no handling at the moment of a click outside the popover, basically if we open it we can only close it by clicking on the trigger.
- There is no z-index set on the element, maybe it's by choice but this could potentally lead to some cases where the popover would not be visible.
- About the requirements, we set a "width", which makes the example look good, but it's too small, imho, bootstrap sets 276px max width, for example.
- maybe we should increase the line-height of the items, when the text wraps in two or more rows it seems maybe too tight.
- we don't handle the scroll, in bootstrap for instance the popover is re-positioned in case it's initial position gets "hidden" by scrolling the page.
- Not sure whether the position of the icon in an item of a list should always be "before", if that is the case maybe it's in the template that this should be "hardcoded".
@planctus I share your concerns about this components. It has been defined with one specific use case in mind: the social share component. It should be extended to other use cases, but that's still blurry
- Yes, only the vertical spacing is taken into account
- For now the only foreseen content is a list of link, but having a block already would make it easier to opens up
- The designers specifically determined that it is a link that should trigger the popover. But I won't be surprised if we have to accept buttons in a near future... Also, having the trigger in the component makes it behave similary to the expandable component
- That's not in the requirements, and honestly I would like to avoid it if possible, as it implies having some clieck event detection for the whole window
- That may be a issue indeed, I guess that we should put a bigger z-index by default
- The width comes from the specs. We can always change it later if needed
- That's true. To be seen with designer what line height we should have
- I do hope that we won't have to develop something as complex as Bootstrap for this. Let's try to keep it simple for now
- That's not specified, but I guess that we could force it to be before indeed
The "content" control doesn't have a default value, even an empty string, but the problem is that currently if we set a "content" this doesn't get styled properly, the popover should have its own internal padding, i believe, instead of delegating that to the internal items.
Regarding the choice of making this an "independent" template that includes the toggle and the content, i'm not really sure that this is going to help, for instance we know that this should be used in the social media, would that mean that we would go including this popover template in these two templates..? To me the template should be included in the link component based on a parameter, the toggler is simply a link, regarding the "consistency" with the expandable, to be honest it feels that also there the expandable should just be a functionality associated to the button, not needing a specific include of a different template, but that is there since years, so...
I'll check the control. But for the integration into link component, my fear is that tomorrow COMM may decide that buttons could have a popover too (or it would be required by Plateform). And so we would have to refactor the template, and most probably introduce breaking changes
i don't understand the concerns, honestly, i do expect comm to ask for the "popover" support in the button, because it makes sense, but once it does that if we have a template for the "popover" only, then this template can be simply applied (included) in any other, it would not introduce any breaking change being a new feature. I really don't understand how this would be currently used, instead, if we make it a standalone component then in the implementation they will not be able to use this functionality as such, they can only "include" the template in some custom, specific field, but to me this is a strong limitation and potentially a blocker when it comes to the social media components. We are adding this to support those components, for instance, but at the moment we would not be able to use it ourselves, we have to add a specific include in both the components, although they are simply defining and including the link template for all the items.
Ok I see it now. I thought you were suggesting to put the popover (css and js) inside the link component. I'll do some test