-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add feature flag for blocked repositories UI #11304
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
Conversation
started the job as gitpod-build-af-feature-flag-for-blocked-repos-ui.11 because the annotations in the pull request description changed |
@andrew-farries When loading
Other errors cite Also, I noticed ☁️ As much as I love the "feature flag in context" approach: One way to move forward with this would be to do the feature flag it "as usual", and play around with the other approach separately. 💯 |
The errors you were seeing were due to me using a telepresence pod at the same time that Werft deployed another dashboard pod to the preview environment. I had the same problem. I've stopped messing with telepresence so it should be good now. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Code LGTM, tested and works.
/hold because of names, names, names... #11304 (comment) 🙃
Renamed and tagged in @gtsiolis because I made the submenu wider to accommodate the longer text. @gtsiolis this width change 31b599f applies to all submenus, not just the admin one. Looking at the other areas where a submenu is used it seems fine. We could alternatively pass the width as a prop to |
Looking at this now! 👀 Thanks for the ping, @andrew-farries! 🏓 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again for the ping, @andrew-farries! 🏓
Left one non-blocking comment regarding menu order. ❗
🍊 🍊 🍊 🍊
The subtle 16px
change in the width of the sidebar sounds ok from UX perspective to leave as is for all sidebar instances, no need to introduce a prop for conditionally change the width. ✔️
This would also look ok even after replacing the sidebar menu with tabs for the admin dashboard #7879. ✔️
Long-term we can work of fine tuning breakpoints and column layout, see relevant discussion (internal) and some early designs around this in #8133 (comment). 🗺️
🍋 🍋 🍋 🍋
One aspect that is not clear but probably irrelevant and out of the scope of this PR at the moment is whether we'd like to also list repositories, introduce a repository view, and have an action to block from there. This could be relevant when we introduce better control for multi-repository support, etc.
🥝 🥝 🥝 🥝
Regarding the menu label, I agree with @geropl in #11304 (comment) on being more explicit here although the dashboard design so far has been using shorter verbs or labels when possible to be clearer to users but also follow our design principles (internal). 🥯
31b599f
to
405aaae
Compare
/unhold |
Description
Add a feature-flagged entry in the admin sidebar that will show a placeholder blocked repositories admin UI.
The feature flag is enabled for non-production environments (ie preview and staging).
Related Issue(s)
Part of #11030
How to test
Start the preview environment and see the new
Repositories
menu item in theAdmin
menu:The link takes you to an empty placeholder page:
Release Notes
Documentation
Werft options: