view_components icon indicating copy to clipboard operation
view_components copied to clipboard

Published 20 hours ago • verified publisher icon primer

Overlay component

Open keithamus opened this issue 2 years ago • 1 comments

This is the initial implementation of the Primer::Alpha::Overlay component done in https://github.com/primer/view_components/pull/1214.

This is largely based off of Primer::Alpha::Dialog but with various tweaks to improve ergonomics and make it more generically applicable to overlays. In addition it builds upon the work we've done in https://github.com/oddbird/popup-polyfill to build a polyfill for the popup API.

This doesn't implement the oddbird polyfill, and instead ships a custom element in order to provide the same popup APIs. This will allow us to expedite shipping this, and work on migrating to the polyfill piecemeal.

keithamus avatar Sep 23 '22 15:09 keithamus

🦋 Changeset detected

Latest commit: 17fe2ed62ce8443c3d1f72615baf974d2420112c

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

This PR includes changesets to release 1 package
Name Type
@primer/view-components 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

changeset-bot[bot] avatar Sep 23 '22 15:09 changeset-bot[bot]

⚠️ Visual differences found

Our visual comparison tests found UI differences. Please review the differences by viewing the files changed tab to ensure that the changes were intentional.

Review visual differences

github-actions[bot] avatar Nov 17 '22 15:11 github-actions[bot]

Do you have any thoughts on adding responsive behavior? We have cases where an ActionMenu may become a bottom sheet or full screen menu on a smaller device. Can we include the same responsive behavior with placement as we have in Dialog? https://github.com/github/primer/blob/main/apis/overlay-api.md#overlaybase

Happy to do this but - I think we discussed this in a pairing - the overlay doesn't have a backdrop, and these classes rely on grid positioning the backdrop to set the anchor position. The classes would need to be refactored to use absolute positioning on the anchor element itself, rather than positioning via the backdrop element. So a larger refactor of the CSS classes will be needed.

keithamus avatar Feb 16 '23 12:02 keithamus

  • Not certain why, noticed the pages doc 404's https://primer-7f6c170b7d-26611710.drafts.github.io/components/alpha/overlay
  • http://primer-view-components-preview-1401.eastus.azurecontainer.io/view-components/lookbook/inspect/primer/alpha/overlay/middle_of_page is giving me Uninitialized constant Primer::Alpha::Overlay::DEFAULT_PADDING Did you mean? Primer::Alpha::Overlay::DEFAULT_SIZE
  • Is the title supposed to be center aligned like this? image

jonrohan avatar Feb 16 '23 17:02 jonrohan

I believe this is good to go now :smile:

keithamus avatar Feb 20 '23 18:02 keithamus

Testing something with the preview deploy, I'm gonna close this and reopen it. I want to see if that fixes the weird asset caching

jonrohan avatar Feb 22 '23 22:02 jonrohan

Its still not working right for me when I test the deploy 😦 is it just me?! Am I cursed?

https://user-images.githubusercontent.com/18661030/220986224-ad0c7fab-f1a8-478c-9a40-eedc9639c3da.mp4

langermank avatar Feb 23 '23 17:02 langermank