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

Adding a tooltip w/ full expiration date #15253

Merged
merged 3 commits into from
Dec 9, 2022
Merged

Conversation

selfcontained
Copy link
Contributor

@selfcontained selfcontained commented Dec 8, 2022

Description

Adds a tooltip w/ the full expiration date of a Personal Access Token for expired tokens.

<Tooltip/> Updates

The positioning currently used for tooltips results in some potential inherited styling issues. For the PAT expired tooltip, it wasn't displaying due to a parent container using the truncate class (which it needs to and is ok). A class of these types of bugs can be resolved by rendering the Tooltip outside of the DOM hierarchy the of the React component structure via React Portals (I used react-portal for this). This appends the node to the document.body. I also used react-popper, which assists in calculating the positioning of the tooltip in comparison to the target that shows it on hover. The react-datepicker library we use also uses react-popper, and I've used it in the past and been happy with it.

Related Issue(s)

Fixes #15103

How to test

Preview Env: https://bmh-pat-ex451962a03f.preview.gitpod-dev.com/tokens
(latest build seems to be taking a long time, not sure if there's a way to cancel/restart a new one?)

This is slightly tricky to test, as you need an expired personal access token. I wasn't sure if there was a way to expire them manually, so I ended up negating the flag here manually as I was developing.

I did verify all of the other areas that use our <Tooltip/> component, such as the Workspace list, Members list, and a few others. Creating a new token, or regenerating one also has an input with a copy icon that has a tooltip that can be tested.

Release Notes

Expired Personal Access Tokens exclamation indicator now has a tooltip w/ the full expiration date so you can see exactly when it expired.

Documentation

Werft options:

  • /werft with-local-preview
    If enabled this will build install/preview
  • /werft with-preview
  • /werft with-large-vm
  • /werft with-integration-tests=all
    Valid options are all, workspace, webapp, ide, jetbrains, vscode, ssh

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-bmh-pat-exp-date-tooltip.2 because the annotations in the pull request description changed
(with .werft/ from main)

@roboquat roboquat added size/L and removed size/XS labels Dec 9, 2022
@selfcontained selfcontained marked this pull request as ready for review December 9, 2022 02:16
@selfcontained selfcontained requested a review from a team December 9, 2022 02:16
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Dec 9, 2022
<span className={"flex items-center gap-1 truncate" + (expired ? " text-orange-600" : "")}>
<span>{expirationDay.format("MMM D, YYYY")}</span>
{expired && (
<Tooltip content={expirationDateString}>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This is the only real change in this block, but I removed the unnecessary fragment wrapper (<></>).

Copy link
Contributor

Choose a reason for hiding this comment

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

Indeed, thanks for the pointer! 👍 (FYI, to review changes with big indentation changes, I like to ignore whitespaces by appending ?w=1 at the end of the diff URL -- for example https://github.com/gitpod-io/gitpod/pull/15253/files?w=1 )

@jankeromnes
Copy link
Contributor

jankeromnes commented Dec 9, 2022

(latest build seems to be taking a long time, not sure if there's a way to cancel/restart a new one?)

I don't know how to cancel an existing build, but you can always trigger new builds by posting a simple comment instruction (see below for an example).

Also, it seems that your latest build https://werft.gitpod-dev.com/job/gitpod-build-bmh-pat-exp-date-tooltip.3 did not finish successfully -- if you click on "Raw Logs" (top right), and scroll all the way to the bottom, it looks like your deployment had some pods that never became "ready", so your build eventually timed out.

I'll manually trigger a new build in the hope that it works now. This can be done by adding the /werft run instruction in a GitHub comment, just like so:

/werft run

👍 started the job as gitpod-build-bmh-pat-exp-date-tooltip.4
(with .werft/ from main)

@easyCZ
Copy link
Member

easyCZ commented Dec 9, 2022

/werft run

👍 started the job as gitpod-build-bmh-pat-exp-date-tooltip.5
(with .werft/ from main)

Copy link
Member

@easyCZ easyCZ left a comment

Choose a reason for hiding this comment

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

Works nicely

Screenshot 2022-12-09 at 14 45 46

@easyCZ
Copy link
Member

easyCZ commented Dec 9, 2022

You can create expired tokens by adjusting the params sent to the API. We don't reject expired tokens as they naturally are immediately rejected when authenticating.

With the pub api, that's easy as you can copy the CreatePersonalAccessToken request as cURL and adjust it

Copy link
Contributor

@jankeromnes jankeromnes left a comment

Choose a reason for hiding this comment

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

Looks good to me, and works like a charm! ✨

Portal and Popper do seem like useful tools -- thanks for bringing them in! 👍

It also seems easier now to mouse into the tooltip and to select its text (e.g. to copy a message)

@gtsiolis
Copy link
Contributor

gtsiolis commented Dec 9, 2022

Holding to unblock merge queue.

/hold

@selfcontained
Copy link
Contributor Author

/unhold

@roboquat roboquat merged commit 90b11f0 into main Dec 9, 2022
@roboquat roboquat deleted the bmh/pat-exp-date-tooltip branch December 9, 2022 16:46
@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note size/L team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improve expiration time UX
5 participants