feat: add auth styling and 'extra actions' menu #3

Parkreiner · 2024-02-21T01:06:41Z

Closes #1. First major commit for the new repo.

Screen.Recording.2024-02-20.at.7.35.50.PM.mov

Changes made

Adds a `CoderAuthWrapper component
Removes the RefreshButton component in favor of an ExtraActionsButton component. Moves the old refresh functionality and the new eject token functionality under that menu
Updates parts of the CoderAuth API to compute the initializing status more accurately

Notes

Renamed logout to ejectToken to make it more clear that calling it does not log the user out of the whole Backstage deployment

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthInputForm.tsx

Kira-Pilot · 2024-02-21T20:25:38Z

Could we add an onKeyDown to our SearchBox component so that when I hit Enter, the search is initiated? Right now, the kebab menu opens instead - maybe because it's the first actionable element?

Kira-Pilot · 2024-02-21T21:02:02Z

I think the VisuallyHidden component's forceShow state might have a unexpected UI for anyone developing on a MacOS. I'm stuck in the below state when I open devtools in Chrome, I guess because Alt maps to Option?
Maybe this was intended but the result is a little visually jarring.

code-asher

If my deployment is down I get an infinite loop of requests to /me, does anyone else see that?

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthWrapper.tsx

code-asher · 2024-02-21T18:36:09Z

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthWrapper.tsx

+  },
+}));
+
+function CoderAuthCard({ children }: PropsWithChildren<unknown>) {


The auth card is looking slick!

code-asher · 2024-02-21T18:36:38Z

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthDistrustedForm.tsx

+      <div>
+        <CoderLogo className={styles.coderLogo} />
+        <p>
+          Unable to verify token authenticity. Please check your internet


I like this! Having the user choose whether to discard the token or not makes a lot of sense.

Kira-Pilot · 2024-02-21T21:28:37Z

If my deployment is down I get an infinite loop of requests to /me, does anyone else see that?

@code-asher I do not but I'm testing against dev.coder.com. Is there an easy way to repo?

Parkreiner · 2024-02-21T21:36:56Z

I think the VisuallyHidden component's forceShow state might have a unexpected UI for anyone developing on a MacOS. I'm stuck in the below state when I open devtools in Chrome, I guess because Alt maps to Option? Maybe this was intended but the result is a little visually jarring.

@Kira-Pilot So, it is a little jarring (especially since some of the visually-hidden labels are a bit on the long side), but it is intentional. It gives you a quick way of scanning which icons do and don't have accessible text associated with them (which is normally impossible to eyeball, since the elements are supposed to be invisible)

Unless you mean that just opening the devtools at all does that, in which case, that definitely isn't intentional

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthInputForm.tsx

Kira-Pilot · 2024-02-21T21:41:25Z

Unless you mean that just opening the devtools at all does that, in which case, that definitely isn't intentional

This is what I mean :)
I like the idea overall though!

Parkreiner · 2024-02-21T21:56:02Z

Could we add an onKeyDown to our SearchBox component so that when I hit Enter, the search is initiated? Right now, the kebab menu opens instead - maybe because it's the first actionable element?

That's definitely a bug. Off the top of my head, I'm not sure how it's happening – even when the event from the input bubbles up, it should miss the button for opening the menu, but I'll get this fixed

Kira-Pilot · 2024-02-21T21:58:04Z

Very small details:

Can we add some padding to this helper text which gets smooshed on smaller screens?
🤔 is 'searchbar' one word? I feel like I usually see a space but I might be missing something.
Can we use a cursor: pointer style on the clear search icon?

Kira-Pilot · 2024-02-21T21:59:30Z

Looking lovely! It's really coming together! 🚀

code-asher · 2024-02-21T22:25:39Z

@code-asher I do not but I'm testing against dev.coder.com. Is there an easy way to repo?

What I do is edit app-config.yaml and change the proxy endpoint to https://localhost:4000 then restart yarn dev. Any random endpoint will work though, as long as there is nothing actually listening there so that the requests fail.

Kira-Pilot · 2024-02-21T22:52:02Z

What I do is edit app-config.yaml and change the proxy endpoint to https://localhost:4000 then restart yarn dev. Any random endpoint will work though, as long as there is nothing actually listening there so that the requests fail.

Oh yeahhhh i see that

Parkreiner · 2024-02-22T19:11:40Z

@Kira-Pilot Almost all your points of feedback should be addressed. The only one that should be left is the VisuallyHidden toggle. I've always been opening it up with F12, so I didn't realize there were shortcuts involving Alt/Option. Not sure if it'll make it in this PR, but I will see if there might be a better way to customize which key triggers things

@code-asher And the last thing from your feedback should be the infinite loop when a deployment's down. Looking at that right now

Parkreiner · 2024-02-22T23:17:59Z

One last set of additions I had to add to the PR: the majority of the tests had to be updated to make sure they don't flake out.

Turns out, all components that you directly wire up to a Backstage plugin and export through it have a lot more metadata attached to them. And depending on the component or the type of functionality you're exposing, you might need to bring in a whole test app to provide all the dependencies. But it's a toss-up, and components don't always fail consistently

Took me a bit of time to track down the issue, but all the test helpers have been updated to use wrapInTestApp, so we shouldn't need to worry about this again. Only change is that the functions will need to be async when they weren't before.

We weren't running into this issue with the previous repo because not all the components were going through this process yet. This is 100% on me for not making sure the exports weren't fully squared away and were going through the plugin

Parkreiner · 2024-02-23T14:18:11Z

plugins/backstage-plugin-coder/src/testHelpers/setup.tsx

+      wrapInTestApp(
+        <TestApiProvider
+          apis={[
+            [errorApiRef, mockErrorApi],
+            [scmIntegrationsApiRef, mockSourceControl],
+            [configApiRef, mockConfigApi],
+          ]}
        >
-          <EntityProvider entity={mockEntity}>{children}</EntityProvider>
-        </CoderProviderWithMockAuth>
-      </TestApiProvider>
-    ),
+          <CoderProviderWithMockAuth
+            appConfig={mockAppConfig}
+            queryClient={mockQueryClient}
+            authStatus={authStatus}
+          >
+            <EntityProvider entity={mockEntity}>
+              <>{children}</>
+            </EntityProvider>
+          </CoderProviderWithMockAuth>
+        </TestApiProvider>,
+      ),


Code is almost exactly the same – only change is that now it's wrapped inside wrapInTestApp

Parkreiner · 2024-02-23T16:26:51Z

plugins/backstage-plugin-coder/src/components/CoderProvider/CoderAuthProvider.tsx

      }
+
+      isRefetchingTokenQuery = true;
+      await queryClient.refetchQueries({ queryKey: authQueryKey });


One important detail: even though refetchQueries is an async function, it never throws. The promise only indicates when a query is done, even if it's not successful, so there's no need for a try/catch

Kira-Pilot

LGTM! Nice job figuring out the test flakes

feat: add auth styling and 'extra actions' menu

5724aee

Parkreiner requested review from Kira-Pilot and code-asher February 21, 2024 01:06

Parkreiner self-assigned this Feb 21, 2024

Parkreiner added 2 commits February 21, 2024 15:34

fix: make sure auth field has valid label

a62bc1d

docs: rewrite comment for clarity

411a017

Kira-Pilot reviewed Feb 21, 2024

View reviewed changes

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthInputForm.tsx Outdated Show resolved Hide resolved

code-asher reviewed Feb 21, 2024

View reviewed changes

Kira-Pilot reviewed Feb 21, 2024

View reviewed changes

plugins/backstage-plugin-coder/src/components/CoderAuthWrapper/CoderAuthInputForm.tsx Outdated Show resolved Hide resolved

Parkreiner added 9 commits February 22, 2024 17:48

refactor: rename reauthenticating status to authenticating

6eeef91

fix: make sure pressing Enter doesn't cause button edge case behavior

b9e9765

fix: clean up form semantics to protect against more edge cases

b5a256e

refactor: rewrite code to have more explicit exhaustiveness checks

6fd74b4

refactor: update style declarations to be less confusing

ad729e1

fix: make sure wrapper is only used when needed

75d410f

fix: make it more clear that clear button is interactive

96b9f7f

fix: update spelling to follow conventional standards

2d0939e

fix: add horizontal padding for search bar reminder

95e483a

fix: prevent infinite loops when proxied deployment is unavailable

705e267

Parkreiner added 4 commits February 22, 2024 19:56

chore: add 'deploymentUnavailable' status to auth

9d9d45d

fix: update test helpers to use newer testing library versions

676c538

fix: polyfill AbortSignal.timeout for tests

c037d8d

fix: resolve all test flakes

c142349

chore: remove unneeded package dependencies

be495b9

Parkreiner requested a review from Kira-Pilot February 23, 2024 14:12

Parkreiner commented Feb 23, 2024

View reviewed changes

Kira-Pilot approved these changes Feb 23, 2024

View reviewed changes

Parkreiner merged commit f2b82a1 into main Feb 23, 2024

Parkreiner deleted the mes/workspaces-list-8 branch February 23, 2024 23:28

feat: add auth styling and 'extra actions' menu #3

feat: add auth styling and 'extra actions' menu #3

Uh oh!

Conversation

Parkreiner commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes made

Notes

Uh oh!

Uh oh!

Kira-Pilot commented Feb 21, 2024

Uh oh!

Kira-Pilot commented Feb 21, 2024

Uh oh!

code-asher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

code-asher Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

code-asher Feb 21, 2024

Choose a reason for hiding this comment

Uh oh!

Kira-Pilot commented Feb 21, 2024

Uh oh!

Parkreiner commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Kira-Pilot commented Feb 21, 2024

Uh oh!

Parkreiner commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kira-Pilot commented Feb 21, 2024

Uh oh!

Kira-Pilot commented Feb 21, 2024

Uh oh!

code-asher commented Feb 21, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Kira-Pilot commented Feb 21, 2024

Uh oh!

Parkreiner commented Feb 22, 2024

Uh oh!

Parkreiner commented Feb 22, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Parkreiner Feb 23, 2024

Choose a reason for hiding this comment

Uh oh!

Parkreiner Feb 23, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Kira-Pilot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Parkreiner commented Feb 21, 2024 •

edited

Loading

Parkreiner commented Feb 21, 2024 •

edited

Loading

Parkreiner commented Feb 21, 2024 •

edited

Loading

code-asher commented Feb 21, 2024 •

edited

Loading

Parkreiner commented Feb 22, 2024 •

edited

Loading

Parkreiner Feb 23, 2024 •

edited

Loading