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

ci: Hide the github button in e2e tests (no-changelog) #11747

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

netroy
Copy link
Member

@netroy netroy commented Nov 14, 2024

Summary

Related Linear tickets, Github issues, and Community forum posts

Review / Merge checklist

  • PR title and summary are descriptive. (conventions)
  • Docs updated or follow-up ticket created.
  • Tests included.
  • PR Labeled with release/backport (if the PR is an urgent fix that needs to be backported)

@netroy netroy force-pushed the hide-github-botton-in-e2e-tests branch from a27a70a to c741b8d Compare November 14, 2024 17:44
@@ -184,15 +187,16 @@ async function navigateToExecutionsView(openInNewTab: boolean) {
@update:model-value="onTabSelected"
/>
</div>
<div class="github-button">
<div class="github-button" v-if="showGitHubButton">
Copy link
Contributor

Choose a reason for hiding this comment

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

can we keep it? Would be great if we can keep e2e tests as close to reality of what users experience as possible

Copy link
Member Author

Choose a reason for hiding this comment

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

I think we should bring it back once/if we decide to move this to a place where it's not so obstructive. In it's current location it's messing with too many e2e tests for my liking.

Copy link
Member Author

Choose a reason for hiding this comment

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

also, for us to keep this, we need a wider viewport (which also doesn't always work). To keep the user experience as close to reality as possible, we should also not be assuming a viewport width of 1920px either.
Even I don't have that wide of a viewport
image

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, thank you for the viewport changes. Would be great to get that in.

I am still not sure about removing Github button. Based on the thread on Slack, it does not seem Product is interested in removing it.

Copy link
Member Author

Choose a reason for hiding this comment

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

we are not removing the button for any other context besides e2e tests. and we probably can't revert the viewport changes, since the button is taking quite a significant horizontal space, which will likely lead to flaky tests.

Copy link
Contributor

Choose a reason for hiding this comment

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

Ok. Maybe it should also be behind an env variable, but we can wait till users start asking for this.

@n8n-assistant n8n-assistant bot added core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system labels Nov 14, 2024
Copy link

codecov bot commented Nov 14, 2024

Codecov Report

Attention: Patch coverage is 83.33333% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
packages/cli/src/services/frontend.service.ts 0.00% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
core Enhancement outside /nodes-base and /editor-ui n8n team Authored by the n8n team ui Enhancement in /editor-ui or /design-system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants