Popover: new element contribution #588

michael-iden · 2023-08-18T18:54:28Z

This change: (check at least one)

Is this a breaking change? (check one)

Yes
No

Is the: (complete all)

Title of this pull request clear, concise, and indicative of the issue number it addresses, if any?
Test suite(s) passing?
Code coverage maximal?
Changeset added?
Component status page up to date?

What does this change address?
Adds a popover element to pharos that fills the role of non-modal dialog

How does this change work?
Adds new popover element that is based heavily off of the existing dropdown-menu component but with relaxed requirements so that any contents can be slotted

Additional context

changeset-bot · 2023-08-18T18:54:31Z

🦋 Changeset detected

Latest commit: 25314cd

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@ithaka/pharos	Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

github-actions · 2023-08-18T18:56:10Z

size-limit report 📦

Path	Size
packages/pharos/lib/index.js	49.76 KB (+3.28% 🔺)

…a-label for dialog

packages/pharos/src/components/popover/PharosPopover.react.stories.jsx

packages/pharos/src/components/popover/pharos-popover.scss

packages/pharos/src/components/popover/PharosPopover.react.stories.jsx

…dius var Co-authored-by: Dane Hillard <github@danehillard.com>

packages/pharos/src/components/popover/pharos-popover.ts

…ents on different backgrounds

daneah · 2023-08-22T12:33:11Z

packages/pharos/src/components/popover/PharosPopover.react.stories.jsx

+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>
+            <div>Some really cool stuff</div>


We typically put some slightly higher fidelity content into these for demonstrative purposes. Maybe @lorilundy711 can make a suggestion here that is in keeping with other components' content?

I converted this to a paragraph of lorem ipsum text as a start but can do whatever if we want different content in there

daneah · 2023-08-22T12:33:56Z

packages/pharos/src/components/popover/pharos-popover.scss

+  display: block;
+  position: fixed;
+  z-index: 140;
+  pointer-events: none;


This is often a red flag for accessibility; I've added @sirrah-tam to review

Not sure if desirable, but I stole this from the dropdown-menu css

I think as long as the content is inline in the DOM from the triggering element, the use of z-index and position: fixed seem okay to me. Depending on the visual layout, we just want to make sure it matches the expected reading order.

packages/pharos/src/components/popover/pharos-popover.scss

daneah · 2023-08-22T12:36:42Z

packages/pharos/src/components/popover/pharos-popover.scss

+  list-style-type: none;
+  padding-left: 0;
+  padding-inline-start: 0;
+  box-shadow: 0 1px 2px rgb(18 18 18 / 0.3), 0 4px 8px 3px rgb(18 18 18 / 0.15);


@lorilundy711 I think this is the first time we've introduced height to a component, is there any concern with the inconsistency here compared to e.g. dropdown menus?

packages/pharos/src/components/popover/pharos-popover.scss

chrisjbrown

tested in chromatic storybook. worked as i expected 👍

sirrah-tam

This looks good to me! The popover was perceived as a dialog and the user was able to navigate through it's content without issue. Users are able to perceive the name of the dialog based on its supplied label attribute and focus is moved to the first interactive element (often close button).

@sirrah-tam

* develop: chore: version packages (#611) Popover: Border radius styling enforcement (#610) Sheet: Component Contribution (#603) docs: add @sirrah-tam as a contributor (#607) Tokens: improve glacier-blue-40 AAA color contrast (#606) chore: bump dependencies and update resolutions (#605) Dropdown: add line rules for light variants (#599) chore: version packages (#601) feat(icon): add compare and side-panel icons (#596) chore(sidenav): fix flaky test (#600) chore: version packages (#595) Popover: new element contribution (#588)

* feat(popover): initial commit of the new popover component * feat(popover): iterating on popover in storybook * feat(popover): allow popover to show and dismiss * feat(popover): add more examples for a popover * feat(popover): clean up component * feat(popover): adding react examples * feat(popover): add changeset * feat(popover): add unit tests * feat(popover): rename unit test names and vars * feat(popover): fix tests initialization, remove unused tests, add aria-label for dialog * feat(popover): simplify template rendering * feat(popover): remove shadow when on background * feat(popver): fix inline style of react storybook and use existing radius var Co-authored-by: Dane Hillard <github@danehillard.com> * feat(popover): update inlines styles for web component storybook * feat(popover): update inlines styles for web component storybook * feat(popover): use storybook dark background * feat(popover): update shadow to latest * feat(popover): fix radius definition * feat(popover): add additional scenario to cover popups with dark contents on different backgrounds * feat(popover): fix focus logic for popover and remove old styles * feat(popover): convert placeholder copy to lorem ipsum * feat(popover): accessibility tweaks for pharos popover * feat(popover): update stories to use label --------- Co-authored-by: Dane Hillard <github@danehillard.com> Co-authored-by: Jialin He <38861633+jialin-he@users.noreply.github.com>

michael-iden added 3 commits August 18, 2023 11:38

feat(popover): initial commit of the new popover component

b134fc2

feat(popover): iterating on popover in storybook

4b833fa

feat(popover): allow popover to show and dismiss

1403634

michael-iden added the status/work in progress label Aug 18, 2023

michael-iden added the component/popover label Aug 18, 2023

michael-iden added 6 commits August 18, 2023 14:58

feat(popover): add more examples for a popover

6ec7283

feat(popover): clean up component

bca2b6e

feat(popover): adding react examples

2244c29

feat(popover): add changeset

5c95dcc

feat(popover): add unit tests

74636b0

feat(popover): rename unit test names and vars

1438422

michael-iden marked this pull request as ready for review August 18, 2023 20:16

michael-iden requested a review from a team as a code owner August 18, 2023 20:16

michael-iden requested review from brentswisher, SMQuazi and mtorres3 and removed request for a team August 18, 2023 20:16

michael-iden removed the status/work in progress label Aug 18, 2023

michael-iden added 4 commits August 21, 2023 10:24

feat(popover): fix tests initialization, remove unused tests, add ari…

a55bb5b

…a-label for dialog

feat(popover): simplify template rendering

9e99396

Merge branch 'develop' into feature/popover

82b40b7

feat(popover): remove shadow when on background

f38cf2e

michael-iden requested a review from jialin-he August 21, 2023 16:54

michael-iden added status/code review status/needs UX review ithaka/pharos labels Aug 21, 2023

daneah reviewed Aug 21, 2023

View reviewed changes

feat(popver): fix inline style of react storybook and use existing ra…

04ac5d7

…dius var Co-authored-by: Dane Hillard <github@danehillard.com>

michael-iden added 5 commits August 21, 2023 13:00

feat(popover): update inlines styles for web component storybook

3488823

feat(popover): update inlines styles for web component storybook

031afcd

feat(popover): use storybook dark background

c808332

feat(popover): update shadow to latest

c171a78

feat(popover): fix radius definition

a4e421d

daneah requested a review from sirrah-tam August 22, 2023 12:32

chrisjbrown reviewed Aug 22, 2023

View reviewed changes

packages/pharos/src/components/popover/pharos-popover.ts Outdated Show resolved Hide resolved

feat(popover): add additional scenario to cover popups with dark cont…

0f562e1

…ents on different backgrounds

daneah reviewed Aug 22, 2023

View reviewed changes

michael-iden added 2 commits August 22, 2023 12:14

feat(popover): fix focus logic for popover and remove old styles

9c41eb8

feat(popover): convert placeholder copy to lorem ipsum

681b9bc

chrisjbrown approved these changes Aug 22, 2023

View reviewed changes

jialin-he approved these changes Aug 22, 2023

View reviewed changes

michael-iden added 2 commits August 23, 2023 12:22

feat(popover): accessibility tweaks for pharos popover

e92346a

feat(popover): update stories to use label

246c010

sirrah-tam approved these changes Aug 23, 2023

View reviewed changes

Merge branch 'develop' into feature/popover

25314cd

sirrah-tam approved these changes Aug 24, 2023

View reviewed changes

michael-iden added status/ready to merge and removed status/needs UX review status/code review labels Aug 29, 2023

jialin-he merged commit 130ce55 into develop Aug 29, 2023
8 checks passed

jialin-he deleted the feature/popover branch August 29, 2023 12:57

github-actions bot mentioned this pull request Aug 29, 2023

Version Packages #595

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Popover: new element contribution #588

Popover: new element contribution #588

michael-iden commented Aug 18, 2023 •

edited

Loading

changeset-bot bot commented Aug 18, 2023 •

edited

Loading

github-actions bot commented Aug 18, 2023 •

edited

Loading

daneah Aug 22, 2023

michael-iden Aug 22, 2023

daneah Aug 22, 2023

michael-iden Aug 22, 2023

sirrah-tam Aug 22, 2023

daneah Aug 22, 2023

chrisjbrown left a comment

sirrah-tam left a comment

Popover: new element contribution #588

Popover: new element contribution #588

Conversation

michael-iden commented Aug 18, 2023 • edited Loading

changeset-bot bot commented Aug 18, 2023 • edited Loading

🦋 Changeset detected

github-actions bot commented Aug 18, 2023 • edited Loading

size-limit report 📦

daneah Aug 22, 2023

Choose a reason for hiding this comment

michael-iden Aug 22, 2023

Choose a reason for hiding this comment

daneah Aug 22, 2023

Choose a reason for hiding this comment

michael-iden Aug 22, 2023

Choose a reason for hiding this comment

sirrah-tam Aug 22, 2023

Choose a reason for hiding this comment

daneah Aug 22, 2023

Choose a reason for hiding this comment

chrisjbrown left a comment

Choose a reason for hiding this comment

sirrah-tam left a comment

Choose a reason for hiding this comment

michael-iden commented Aug 18, 2023 •

edited

Loading

changeset-bot bot commented Aug 18, 2023 •

edited

Loading

github-actions bot commented Aug 18, 2023 •

edited

Loading