Skip to content

flag: add BETA Label to /tokens page #15201

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 7, 2022
Merged

flag: add BETA Label to /tokens page #15201

merged 3 commits into from
Dec 7, 2022

Conversation

Siddhant-K-code
Copy link
Member

@Siddhant-K-code Siddhant-K-code commented Dec 7, 2022 •
edited
Loading

Signed-off-by: Siddhant Khare siddhant@gitpod.io

Description

This PR adds a Beta Label to /tokens page.

Preview (Light) Preview(Dark)
image image

Related Issue(s)

Fixes #internal conversation

How to test

  • Open Preview
  • Go To /tokens & see 👀 (if feature flag is enable for you)

Release Notes

NONE

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

@Siddhant-K-code Siddhant-K-code requested a review from a team December 7, 2022 06:46
@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-label-pat-menu.1 because the annotations in the pull request description changed
(with .werft/ from main)

@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Dec 7, 2022
@easyCZ
Copy link
Member

easyCZ commented Dec 7, 2022

Screenshot 2022-12-07 at 8 43 27

Should the label be on the second row? I think it should be next to the header

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-label-pat-menu.3 because the annotations in the pull request description changed
(with .werft/ from main)

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-label-pat-menu.4 because the annotations in the pull request description changed
(with .werft/ from main)

loujaybee
loujaybee approved these changes Dec 7, 2022
View reviewed changes
Copy link
Member

@loujaybee loujaybee left a comment

Choose a reason for hiding this comment

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

Approving from a product perspective, but would prefer an engineer / WebApp approval before merge !

/hold

gtsiolis
gtsiolis reviewed Dec 7, 2022
View reviewed changes
Copy link
Contributor

@gtsiolis gtsiolis left a comment

Choose a reason for hiding this comment

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

Should the label be on the second row? I think it should be next to the header

+1 — Yes, please!

@easyCZ
Copy link
Member

easyCZ commented Dec 7, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-label-pat-menu.6
(with .werft/ from main)

@Siddhant-K-code
Copy link
Member Author

Siddhant-K-code commented Dec 7, 2022 •
edited by werft-gitpod-dev-com bot
Loading

Should the label be on the second row? I think it should be next to the header

+1 — Yes, please!

It's done ✅

@easyCZ
Copy link
Member

easyCZ commented Dec 7, 2022

Will check out the preview, once ready, and then happy to land it.

@Siddhant-K-code @easyCZ
flag: add BETA Label to /tokens page
a1482dc 
Signed-off-by: Siddhant Khare <siddhant@gitpod.io>
@Siddhant-K-code @easyCZ
Label Positionning
17692ef
@Siddhant-K-code
Copy link
Member Author

Siddhant-K-code commented Dec 7, 2022 •
edited by werft-gitpod-dev-com bot
Loading

/werft run

👍 started the job as gitpod-build-label-pat-menu.9
(with .werft/ from main)

@easyCZ
Copy link
Member

easyCZ commented Dec 7, 2022

Screenshot 2022-12-07 at 13 59 53

Could possibly be sitting a bit higher, to be more aligned with the title. But this works for now.

@Siddhant-K-code
Copy link
Member Author

Siddhant-K-code commented Dec 7, 2022 •
edited by werft-gitpod-dev-com bot
Loading

Could possibly be sitting a bit higher, to be more aligned with the title

I had tried that, doesn't seems to get changed as it is coming from the parent class of PillLabel. 😞

@Siddhant-K-code
Copy link
Member Author

can we unhold this one?

@easyCZ
Copy link
Member

easyCZ commented Dec 7, 2022

/unhold

@roboquat roboquat merged commit eff4ffd into main Dec 7, 2022
@roboquat roboquat deleted the label/pat-menu branch December 7, 2022 14:48
This was referenced Dec 7, 2022
Open
Merged
@gtsiolis
Copy link
Contributor

gtsiolis commented Dec 7, 2022

I had tried that, doesn't seems to get changed as it is coming from the parent class of PillLabel.

It's ok, @Siddhant-K-code! This happens because of the default (baseline) alignment.

We could override and decrease the padding of label to improve the visual balance with the heading next to it. We've done similar overrides for the label in usage pages, etc, see #15017. Didn't come here in time[1] but opened the following to change this:

Ideally, we could introduce a new label component variant for smaller labels. Opened the following issue to keep track of these improvements:

@Siddhant-K-code
Copy link
Member Author

Siddhant-K-code commented Dec 8, 2022 •
edited by werft-gitpod-dev-com bot
Loading

Thanks @gtsiolis 🙌🏽

@roboquat roboquat added deployed: webapp Meta team change is running in production deployed Change is completely running in production labels Dec 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@gtsiolis gtsiolis gtsiolis left review comments

@easyCZ easyCZ easyCZ approved these changes

@loujaybee loujaybee loujaybee approved these changes

Assignees
No one assigned
Labels
deployed: webapp Meta team change is running in production deployed Change is completely running in production release-note-none size/XS team: webapp Issue belongs to the WebApp team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@Siddhant-K-code @easyCZ @gtsiolis @loujaybee @roboquat