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.

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

Fixup css #60

Merged
merged 8 commits into from
Jan 16, 2023
Merged

Fixup css #60

merged 8 commits into from
Jan 16, 2023

Conversation

keithamus
Copy link
Collaborator

@keithamus keithamus commented Jan 16, 2023

Description

This changes up some of the CSS to align closer to Chrome's user-agent CSS. What's changed:

  • Simplify selectors to just [popover] as Chrome does.
  • Remove top, left in favour of inset.
  • Drop the inset-*-* properties.
  • Shrink padding from 1em to 0.25em.
  • Drop the 1px default border. Chrome's border-width is initial.
  • Move the z-index into the [popover] selector so it's always applied
  • Rather than [popover] having display: none, and [popver]:open being display: block, we now have [popover]:not(:open) as display: none. This way one can style a popover element to be something like display: grid or display: flex and popover will not override that.
  • Add [popover][anchor] styles, to avoid conflicting with anchored positioning.

Steps to test/reproduce

Please explain how to best reproduce the issue and/or test the changes locally (including the pages/URLs/views/states to review).

Show me

Provide screenshots/animated gifs/videos if necessary.

REMEMBER: Attach this PR to the Trello card

@@ -44,7 +44,6 @@ test('popover as "manual"', async ({ page }) => {

test('popover as "invalid"', async ({ page }) => {
const popover = (await page.locator('#popover6')).nth(0);
await expect(popover).toBeVisible();
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

This expectation has been dropped as it does not hold true in a default Chrome environment.

@jgerigmeyer jgerigmeyer merged commit b45eb36 into main Jan 16, 2023
@jgerigmeyer jgerigmeyer deleted the fixup-css branch January 16, 2023 19:16
@keithamus keithamus mentioned this pull request Jan 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants