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

[dashboard] BlockedRepo UI #11446

Merged
merged 1 commit into from
Jul 19, 2022
Merged

[dashboard] BlockedRepo UI #11446

merged 1 commit into from
Jul 19, 2022

Conversation

geropl
Copy link
Member

@geropl geropl commented Jul 18, 2022

Description

This still looks... ugly, but fixes the general issue of not being able to add blocked repositories from the UI.

Screenshot 2022-07-19 at 11 43 12 AM (2)

Related Issue(s)

Fixes #

How to test

Release Notes

NONE

Documentation

Werft options:

  • /werft with-preview

@werft-gitpod-dev-com
Copy link

started the job as gitpod-build-gpl-blocked-repo-ui.1 because the annotations in the pull request description changed
(with .werft/ from main)

@roboquat roboquat added size/XL and removed size/L labels Jul 18, 2022
@geropl geropl marked this pull request as ready for review July 18, 2022 17:16
@geropl geropl requested a review from a team July 18, 2022 17:16
@github-actions github-actions bot added the team: webapp Issue belongs to the WebApp team label Jul 18, 2022
@geropl
Copy link
Member Author

geropl commented Jul 18, 2022

@AlexTugarev Can you maybe 10mins tomorrow and help me fix the basics for the 🛹 ? 🙏

@gtsiolis I guess you're pretty busy, but: It would be awesome if you could have a look (independent of when we merge this) and come up with a nice version - happy to implement that. ✨

@gtsiolis
Copy link
Contributor

gtsiolis commented Jul 18, 2022

Looking at this now! 👀

@gtsiolis gtsiolis requested a review from a team July 18, 2022 19:22
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.

Thanks for adding this, @geropl! 🔮

Left some comments in case we'd like to address all these UX issues here or in a separate PR.

Approving to unblock merging this, but holding to let someone from the @gitpod-io/engineering-webapp to take a closer look at the code changes. 🏓

/hold

components/dashboard/src/admin/BlockedRepositories.tsx Outdated Show resolved Hide resolved
components/dashboard/src/admin/BlockedRepositories.tsx Outdated Show resolved Hide resolved
components/dashboard/src/admin/BlockedRepositories.tsx Outdated Show resolved Hide resolved
components/dashboard/src/admin/BlockedRepositories.tsx Outdated Show resolved Hide resolved
@geropl
Copy link
Member Author

geropl commented Jul 19, 2022

/unhold

I addressed as much of the suggestions as possible to still get the release train 🚆 😆

To reiterate why I think this is ok:

  • this is admin functionality, so very limited audience, and not in front of (paying) users
  • we should still aim high, but we also want this in prod asap, as it eases on-call 💯
  • none of the trade-offs we took here do affect safety or stability (e.g., even without more validation, the regexes do no harm)
  • The follow-up PR for fixing the UX issues is mandatory 🙃

@roboquat roboquat merged commit 51eae90 into main Jul 19, 2022
@roboquat roboquat deleted the gpl/blocked-repo-ui branch July 19, 2022 07:41
@geropl
Copy link
Member Author

geropl commented Jul 19, 2022

Thank you @andrew-farries and @gtsiolis for your valuable feedback! 🙏

@roboquat roboquat added the deployed: webapp Meta team change is running in production label Jul 19, 2022
@geropl geropl mentioned this pull request Jul 19, 2022
5 tasks
@gtsiolis
Copy link
Contributor

gtsiolis commented Jul 19, 2022

Thanks @geropl! I've updated the screenshot in the PR description to reflect the changes from this PR for future reference, and opened #11463 to summarize the outstanding points mentioned in the discussions above. 🏓

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 release-note-none size/XL team: product-design team: webapp Issue belongs to the WebApp team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants