-
Notifications
You must be signed in to change notification settings - Fork 924
chore: apply design changes to the admin settings menu dropdown #15947
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
} | ||
>; | ||
|
||
export const FeatureStageBadge: FC<FeatureStageBadgeProps> = ({ | ||
contentType, | ||
size = "md", | ||
showTooltip = true, // This is a temporary until the deprecated popover is removed |
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.
showTooltip = true, // This is a temporary until the deprecated popover is removed | |
showTooltip = true, // This is temporary until the deprecated popover is removed |
also, how can we track that this actually gets cleaned up later?
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.
We could create an issue but im not sure its worth the trouble. This will get resolved automatically either when the beta badge is removed or this component moves to the new popover. The issue is that the current popover does not work well when it appears too close to the right edge of the screen.
@@ -109,16 +109,7 @@ const DeploymentDropdownContent: FC<DeploymentDropdownProps> = ({ | |||
onClick={onPopoverClose} | |||
> | |||
Organizations | |||
</MenuItem> | |||
)} | |||
{canViewAllUsers && ( |
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.
I worry that this is going to be one of those things that customers with lots of screenshots will really dislike. Can we loop @bpmct in on this so we can communicate this change clearly and hopefully mitigate frustrations?
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.
Ideally, the figma designs would already have been reviewed to validate this type of change. Almost everything about this PR would change existing screenshots. We should converge on a process after the holidays so we don't have to worry about these types of changes during PR review.
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.
yeah but changing things stylistically is very different from removing a path that users are accustomed to and have documented. I doubt customers care nearly as much when things look slightly different but have the same structure as they do when we remove the button they put a screenshot of in their documentation entirely.
resolves coder/internal#177
Design changes for the admin settings menu dropdown