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

test: add test setup for dev overlay #8932

Merged
merged 5 commits into from
Oct 27, 2023
Merged

Conversation

Princesseuh
Copy link
Member

Changes

Wow, E2E tests are annoying.

Testing

It's all tests!

Docs

N/A

@changeset-bot
Copy link

changeset-bot bot commented Oct 26, 2023

🦋 Changeset detected

Latest commit: 4ae3a59

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

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 github-actions bot added the pkg: astro Related to the core `astro` package (scope) label Oct 26, 2023
@github-actions
Copy link
Contributor

github-actions bot commented Oct 26, 2023

⚖️ Bundle Size Check

Latest commit: 4ae3a59

File Old Size New Size Change
dev-overlay/overlay 25.37 KB 25.47 KB + 100 B
dev-overlay/ui-library/tooltip 6.71 KB 6.81 KB + 101 B
dev-overlay/ui-library/window 5.48 KB 5.48 KB - NaN undefined

@Princesseuh Princesseuh marked this pull request as ready for review October 27, 2023 12:51
Comment on lines +60 to +63
.section-content {
max-height: 250px;
overflow-y: scroll;
}
Copy link
Member Author

Choose a reason for hiding this comment

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

The virtual Playwright window is quite small, and somehow in some cases it couldn't see the tooltip. This is generally just a good change, because the tooltip could get quite long (ex: the docs sidebars)

@@ -30,7 +30,7 @@ export class DevOverlayWindow extends HTMLElement {
font-family: ui-sans-serif, system-ui, -apple-system, BlinkMacSystemFont, "Segoe UI", Roboto, "Helvetica Neue", Arial, "Noto Sans", sans-serif, "Apple Color Emoji", "Segoe UI Emoji", "Segoe UI Symbol", "Noto Color Emoji";
color: rgba(204, 206, 216, 1);
position: fixed;
z-index: 9999999999;
z-index: 999999999;
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 caused the window to appear over the dev overlay, which made it impossible to click.

Copy link
Member

Choose a reason for hiding this comment

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

Might be nice to extract each z-index we need to a custom property at the root of the dev overlay component? Then this would be slightly easier to manage in one place.

If there's a good reason you're not already doing that, ignore me!

Copy link
Member Author

Choose a reason for hiding this comment

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

There's no reason not to, I think. Will refactor in a separate PR!

Copy link
Member

@natemoo-re natemoo-re left a comment

Choose a reason for hiding this comment

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

Looks great! Consider my suggestion non-blocking.

@github-actions github-actions bot added the pr: docs A PR that includes documentation for review label Oct 27, 2023
@Princesseuh Princesseuh merged commit 5fed432 into main Oct 27, 2023
14 checks passed
@Princesseuh Princesseuh deleted the feat/dev-overlay-tests branch October 27, 2023 14:05
@astrobot-houston astrobot-houston mentioned this pull request Oct 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg: astro Related to the core `astro` package (scope) pr: docs A PR that includes documentation for review
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants