feat(popover-beta): Added popover-beta component #642

jason-evans-genesys · 2024-10-04T17:32:38Z

This PR came out of this other one: #627

TODO: add new UX token once UX finishes this ticket: https://inindca.atlassian.net/browse/GDS-2672

Using native popover-api with the popover-beta component. This has a breaking change which is requiring the for prop to be set.

✅ Closes: COMUI-1637

… popover-api ✅ Closes: COMUI-1637

github-actions · 2024-10-04T17:32:51Z

Demo will be published at https://apps.inindca.com/common-ui-docs/genesys-webcomponents/feature/COMUI-1637-3

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx

gavin-everett-genesys · 2024-10-07T10:28:12Z

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.scss

+  position: absolute;
+  top: 0;
+  left: 0;
+  z-index: var(--gse-semantic-zIndex-popover);


It looks by using the native api it adds the popover to the top layer. You can see this by scrolling the container in the example file the popover will appear over all the elements. Whereas in the stable gux-popover it utilises the z-index we set. I am not sure if we need this property anymore but we can maybe discuss in collab time.

From the docs : "Shows a popover element by adding it to the top layer."

https://developer.mozilla.org/en-US/docs/Web/API/Popover_API

Great catch. I'm seeing this in the docs too. I can bring it up during collab time.

gavin-everett-genesys · 2024-10-07T10:38:50Z

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx

+    this.forElement.popoverTargetAction = 'toggle';
+
+    if (this.isOpen) {
+      this.popupElement.showPopover();


From reading through the docs the popupElement should have an attribute of popup for the showPopover to work but I could not find it.

https://developer.mozilla.org/en-US/docs/Web/API/HTMLElement/showPopover

I think it's that the div that has a popover attribute on it should work to use the showPopover method.

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx

gavin-everett-genesys · 2024-10-07T10:45:51Z

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.scss

+  .gux-arrow {
+    position: absolute;
+    width: var(--gse-ui-popover-anchor-width);
+    height: var(--gse-ui-popover-anchor-height);
+
+    /* 4px padding leaves some space for the shadow to bleed into */
+    padding-bottom: 4px;
+    overflow: hidden;


I dont think this arrow is currently displaying.

Looking into this now.

gavin-everett-genesys · 2024-10-07T10:45:57Z

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.scss

+  left: 0;
+  z-index: var(--gse-semantic-zIndex-popover);
+  display: inline-block;
+  min-width: 280px;


is a token needed here ?

Great catch. I'll ask the UX team.

UX said they will make a new token for this: https://inindca.atlassian.net/browse/GDS-2672

gavin-everett-genesys · 2024-10-07T10:52:47Z

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx

+      <popover
+        ref={(el: HTMLDivElement) => (this.popupElement = el)}
+        class={{
+          'gux-hidden': !this.isOpen,


I think this styling can be removed as its probably done via , showPopover and hidePopover I think.

So you're right that we don't need to control visibility anymore with css (I've removed the gux-hidden css for this component) but I'm having trouble checking if the popover is visible or not in this component's unit tests. Checking if gux-hidden is present or not makes it easy to test visibility for this component but let me know if you know of another way to check if the popover is visible or not in jest.

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx

✅ Closes: COMUI-1637

jason-evans-genesys · 2024-10-11T14:50:26Z

@daragh-king-genesys I got the popover to hide when you scroll outside of its context container so all good now: 090b604

katie-bobbe-genesys · 2024-10-14T21:11:33Z

packages/genesys-spark-components/src/components/beta/gux-popover-beta/tests/gux-popover.e2e.ts

+        <div id="popover-target">
+          Example Element
+        </div>
+        <gux-popover position="top" for="popover-target">


Make sure that these tests are updated to test the gux-popover-beta component

katie-bobbe-genesys · 2024-10-14T21:12:09Z

packages/genesys-spark-components/src/components/stable/gux-popover/gux-popover.tsx

@@ -215,6 +215,8 @@ export class GuxPopover {
  componentDidLoad(): void {
    if (this.isOpen) {
      this.runUpdatePosition();
+    } else {


Is this an unrelated change? It should be excluded from this PR

katie-bobbe-genesys · 2024-10-14T21:17:42Z

the feature branch is not building, I think because of a snapshot issue having to do with time zones. If you merge in main the feature branch should build

✅ Closes: COMUI-1637

jason-evans-genesys · 2024-10-15T14:30:30Z

the feature branch is not building, I think because of a snapshot issue having to do with time zones. If you merge in main the feature branch should build

@katie-bobbe-genesys I merged main into this branch. Is the feature branch building for you now?

feat(popover-beta): Added popover-beta component that uses the native…

f34bb69

… popover-api ✅ Closes: COMUI-1637

jason-evans-genesys self-assigned this Oct 4, 2024

github-actions bot requested review from daragh-king-genesys and katie-bobbe-genesys October 4, 2024 17:32

jason-evans-genesys mentioned this pull request Oct 4, 2024

fix(popover): Using native popover-api #627

Closed

gavin-everett-genesys reviewed Oct 7, 2024

View reviewed changes

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx Outdated Show resolved Hide resolved

gavin-everett-genesys reviewed Oct 7, 2024

View reviewed changes

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx Outdated Show resolved Hide resolved

gavin-everett-genesys reviewed Oct 7, 2024

View reviewed changes

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx Outdated Show resolved Hide resolved

gavin-everett-genesys reviewed Oct 7, 2024

View reviewed changes

packages/genesys-spark-components/src/components/beta/gux-popover-beta/gux-popover.tsx Outdated Show resolved Hide resolved

jason-evans-genesys added 6 commits October 7, 2024 10:30

chore(popover-beta): PR feedback

b9aa75e

✅ Closes: COMUI-1637

chore(popover-beta): Using togglePopover method

ac36a96

✅ Closes: COMUI-1637

chore(popover-beta): More PR feedback

df4f6d9

✅ Closes: COMUI-1637

chore(popover-beta): Fixed positioning issue of popover

e8cac12

✅ Closes: COMUI-1637

chore(popover-beta): Fixed tests

96eb1b9

✅ Closes: COMUI-1637

chore(popover-beta): Snapshot test update

5d7a009

✅ Closes: COMUI-1637

jason-evans-genesys requested a review from gavin-everett-genesys October 10, 2024 14:11

jason-evans-genesys added 2 commits October 10, 2024 10:17

chore(popover-beta): Fixed snapshot

c8ef7eb

✅ Closes: COMUI-1637

chore(popover-beta): Using floating-ui hide

090b604

✅ Closes: COMUI-1637

katie-bobbe-genesys reviewed Oct 14, 2024

View reviewed changes

jason-evans-genesys added 2 commits October 15, 2024 10:06

chore(popover-beta): Merged main into branch

f85b0b3

✅ Closes: COMUI-1637

chore(popover-beta): Fixed tests

fa5c7ea

✅ Closes: COMUI-1637

jason-evans-genesys requested a review from katie-bobbe-genesys October 15, 2024 14:30

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(popover-beta): Added popover-beta component #642

feat(popover-beta): Added popover-beta component #642

jason-evans-genesys commented Oct 4, 2024 •

edited

Loading

github-actions bot commented Oct 4, 2024

gavin-everett-genesys Oct 7, 2024 •

edited

Loading

jason-evans-genesys Oct 7, 2024

gavin-everett-genesys Oct 7, 2024

jason-evans-genesys Oct 7, 2024

gavin-everett-genesys Oct 7, 2024

jason-evans-genesys Oct 7, 2024

gavin-everett-genesys Oct 7, 2024

jason-evans-genesys Oct 7, 2024

jason-evans-genesys Oct 7, 2024

gavin-everett-genesys Oct 7, 2024

jason-evans-genesys Oct 7, 2024

jason-evans-genesys commented Oct 11, 2024

katie-bobbe-genesys Oct 14, 2024

jason-evans-genesys Oct 15, 2024

katie-bobbe-genesys Oct 14, 2024

jason-evans-genesys Oct 15, 2024

katie-bobbe-genesys commented Oct 14, 2024

jason-evans-genesys commented Oct 15, 2024

feat(popover-beta): Added popover-beta component #642

Are you sure you want to change the base?

feat(popover-beta): Added popover-beta component #642

Conversation

jason-evans-genesys commented Oct 4, 2024 • edited Loading

github-actions bot commented Oct 4, 2024

gavin-everett-genesys Oct 7, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jason-evans-genesys commented Oct 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

katie-bobbe-genesys commented Oct 14, 2024

jason-evans-genesys commented Oct 15, 2024

jason-evans-genesys commented Oct 4, 2024 •

edited

Loading

gavin-everett-genesys Oct 7, 2024 •

edited

Loading