Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Overlay component #1401

Merged
merged 33 commits into from
Feb 28, 2023
Merged

Overlay component #1401

merged 33 commits into from
Feb 28, 2023

Conversation

keithamus
Copy link
Member

This is the initial implementation of the Primer::Alpha::Overlay component done in #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.

@changeset-bot
Copy link

changeset-bot bot commented Sep 23, 2022 •
edited
Loading

🦋 Changeset detected

Latest commit: 17fe2ed

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

@jonrohan jonrohan temporarily deployed to review-pr-1401 September 30, 2022 17:35 Inactive
@github-actions github-actions bot added javascript Pull requests that update Javascript code ruby Pull requests that update Ruby code labels Sep 30, 2022
@jonrohan jonrohan temporarily deployed to github-pages September 30, 2022 17:39 Inactive
@primer-css primer-css temporarily deployed to review-pr-1401 October 20, 2022 18:57 Inactive
@primer-css primer-css temporarily deployed to github-pages October 20, 2022 19:05 Inactive
@keithamus keithamus force-pushed the initial-overlay-implementation branch from 4dfe3b7 to 40f3649 Compare November 15, 2022 10:58
@keithamus keithamus temporarily deployed to review-pr-1401 November 15, 2022 10:58 Inactive
@keithamus keithamus temporarily deployed to github-pages November 15, 2022 11:03 Inactive
@github-actions github-actions bot added the css Pull requests that update CSS code label Nov 17, 2022
@keithamus keithamus force-pushed the initial-overlay-implementation branch from 7a4f00d to eeeff72 Compare November 17, 2022 15:08
@github-actions
Copy link
Contributor

github-actions bot commented Nov 17, 2022 •
edited
Loading

⚠️ 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

@keithamus keithamus temporarily deployed to review-pr-1401 November 17, 2022 15:29 Inactive
@keithamus keithamus temporarily deployed to github-pages November 17, 2022 15:33 Inactive
@simurai simurai temporarily deployed to review-pr-1401 November 29, 2022 07:52 Inactive
This was referenced Nov 29, 2022
Closed
Merged
@keithamus keithamus force-pushed the initial-overlay-implementation branch from adf56aa to 4e2b3e7 Compare January 10, 2023 11:46
@keithamus keithamus temporarily deployed to review-pr-1401 January 10, 2023 11:46 — with GitHub Actions Inactive
@keithamus keithamus temporarily deployed to github-pages January 10, 2023 11:51 — with GitHub Actions Inactive
@keithamus keithamus temporarily deployed to review-pr-1401 January 19, 2023 15:37 — with GitHub Actions Inactive
@keithamus keithamus temporarily deployed to github-pages February 22, 2023 17:26 — with GitHub Actions Inactive
@jonrohan jonrohan temporarily deployed to preview February 22, 2023 19:45 — with GitHub Actions Inactive
@jonrohan jonrohan temporarily deployed to github-pages February 22, 2023 19:49 — with GitHub Actions Inactive
@jonrohan
Copy link
Member

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 jonrohan closed this Feb 22, 2023
auto-merge was automatically disabled February 22, 2023 22:17

Pull request was closed

@jonrohan jonrohan reopened this Feb 22, 2023
@jonrohan jonrohan temporarily deployed to preview February 22, 2023 22:19 — with GitHub Actions Inactive
@jonrohan jonrohan temporarily deployed to github-pages February 22, 2023 22:22 — with GitHub Actions Inactive
@keithamus keithamus temporarily deployed to preview February 23, 2023 11:10 — with GitHub Actions Inactive
keithamus
keithamus commented Feb 23, 2023
View reviewed changes
test/css/component_selector_use_test.rb Outdated
Comment on lines 91 to 96
try {
if (node.matches(componentSelector) || node.querySelectorAll(`:scope > ${componentSelector}`).length > 0) {
return true;
}
} catch (e) {
return false;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is needed because :open is not currently a valid selector. The CSS parser in the browser will safely ignore this rule, but matches/qSA will throw. Given these are CSS selectors only, we just want to return false instead of throw.

@keithamus keithamus temporarily deployed to github-pages February 23, 2023 11:15 — with GitHub Actions Inactive
@keithamus keithamus temporarily deployed to preview February 23, 2023 12:11 — with GitHub Actions Inactive
@keithamus keithamus temporarily deployed to github-pages February 23, 2023 12:16 — with GitHub Actions Inactive
@keithamus keithamus enabled auto-merge (squash) February 23, 2023 13:53
@langermank
Copy link
Contributor

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

CleanShot.2023-02-23.at.09.37.21.mp4

langermank
langermank approved these changes Feb 28, 2023
View reviewed changes
Copy link
Contributor

@langermank langermank left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚀 Excited to ship this and keep iterating!

@keithamus keithamus merged commit f824d1d into main Feb 28, 2023
@keithamus keithamus deleted the initial-overlay-implementation branch February 28, 2023 16:08
@primer-css primer-css mentioned this pull request Feb 28, 2023
Merged
@keithamus keithamus mentioned this pull request Feb 28, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jonrohan jonrohan jonrohan left review comments

@langermank langermank langermank approved these changes

Assignees
No one assigned
Labels
css Pull requests that update CSS code javascript Pull requests that update Javascript code ruby Pull requests that update Ruby code
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@keithamus @jonrohan @langermank @simurai @primer-css