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

Remove GraphQL / GitHub / Keystone links in production #7546

Merged
merged 3 commits into from
May 23, 2022

Conversation

Achisingh
Copy link
Contributor

@Achisingh Achisingh commented May 20, 2022

The keystone Admin UI popover button that consisted of Keystone links to API Explorer, GitHub Repository and Keystone Documentation, present at the top of the left side-bar is replaced with only the Sign out button in production mode after authentication.
Screenshot 2022-05-23 at 12 54 37 pm

@changeset-bot
Copy link

changeset-bot bot commented May 20, 2022

🦋 Changeset detected

Latest commit: 4c7b471

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

This PR includes changesets to release 15 packages
Name Type
@keystone-6/core Minor
@keystone-6/auth Major
@keystone-6/cloudinary Major
@keystone-6/fields-document Major
@keystone-6/session-store-redis Major
@keystone-6/example-auth Patch
@keystone-6/examples-app-basic Patch
@keystone-6/example-ecommerce Patch
@keystone-6/example-graphql-api-endpoint Patch
@keystone-6/example-roles Patch
@keystone-6/example-sandbox Patch
@keystone-6/example-testing Patch
@keystone-6/example-with-auth Patch
@keystone-6/website Patch
@keystone-6/example-document-field Patch

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

@vercel
Copy link

vercel bot commented May 20, 2022

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Updated
keystone-next-docs ✅ Ready (Inspect) Visit Preview May 23, 2022 at 5:16AM (UTC)

@Achisingh Achisingh changed the title Remove Keystone Admin UI popover button in production and replace with Signout button Replace Keystone Admin UI popover button in production with Sign-out button May 20, 2022
@codesandbox-ci
Copy link

codesandbox-ci bot commented May 20, 2022

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

@Achisingh
Copy link
Contributor Author

Achisingh commented May 20, 2022

Hi @raveling
This is what the UI looks after replacing the popover button with Sign-out in production. As you will see the text is coming very close to the button. Any suggestions on how to go about this?
Screenshot 2022-05-19 at 2 59 49 pm

For reference, this is how the text and the popover button are in dev mode.
Screenshot 2022-05-20 at 11 27 59 am

@vercel vercel bot temporarily deployed to Preview May 23, 2022 02:47 Inactive
@Achisingh
Copy link
Contributor Author

Achisingh commented May 23, 2022

This is how the Signed in as looks like after stylus update
Screenshot 2022-05-23 at 12 46 50 pm

@raveling
Copy link
Contributor

That's acceptable from a Design point of view. Thanks @Achisingh !

emmatown
emmatown previously approved these changes May 23, 2022
@emmatown emmatown dismissed their stale review May 23, 2022 04:03

Missed something

@emmatown
Copy link
Member

Could you update the test at

test('sign out and sign in', async () => {
await page.click('[aria-label="Links and signout"]');
await Promise.all([page.waitForNavigation(), page.click('button:has-text("Sign out")')]);
await page.fill('[placeholder="email"]', 'admin@keystonejs.com');
await page.fill('[placeholder="password"]', 'password');
await Promise.all([page.waitForNavigation(), page.click('button:has-text("Sign In")')]);
});
? The test will need to do different behavior depending on whether it's running the prod vs dev mode. You'll probably want to pass the mode variable to
tests(playwright[browserName]);

@vercel vercel bot temporarily deployed to Preview May 23, 2022 05:16 Inactive
@Achisingh
Copy link
Contributor Author

Could you update the test at

test('sign out and sign in', async () => {
await page.click('[aria-label="Links and signout"]');
await Promise.all([page.waitForNavigation(), page.click('button:has-text("Sign out")')]);
await page.fill('[placeholder="email"]', 'admin@keystonejs.com');
await page.fill('[placeholder="password"]', 'password');
await Promise.all([page.waitForNavigation(), page.click('button:has-text("Sign In")')]);
});

? The test will need to do different behavior depending on whether it's running the prod vs dev mode. You'll probably want to pass the mode variable to

tests(playwright[browserName]);

Done. Refer to:
https://github.com/keystonejs/keystone/pull/7546/commits/4c7b4713526280a6c1ded38c3308813300cb1442

@Achisingh Achisingh merged commit 1944636 into main May 23, 2022
@Achisingh Achisingh deleted the remove-admin-ui-links-prod branch May 23, 2022 05:39
@dcousens dcousens changed the title Replace Keystone Admin UI popover button in production with Sign-out button Remove GraphQL / GitHub / Keystone links in production May 23, 2022
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.

3 participants