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

web: show 'alerts on top' toggle even when there are no tests #4182

Merged

Conversation

landism
Copy link
Member

@landism landism commented Feb 9, 2021

image

I'm not sure if it's poor form to simply set visibility: hidden rather than omit the checkboxes entirely - it's certainly vastly easier than changing a bunch of css to keep the 'alerts on top' toggle in place when the filter checkboxes are absent.

@landism landism requested review from hyu and maiamcc February 9, 2021 16:02
@shortcut-integration
Copy link

Copy link
Contributor

@maiamcc maiamcc left a comment

Choose a reason for hiding this comment

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

I'm not sure if it's poor form to simply set visibility: hidden rather than omit the checkboxes entirely
haaa that is 100% a question for @hyu, but the code logic looks fine and i'd feel good about merging this and letting Han go back over the styling after if needful, cuz imho it looks good enough(tm).

web/src/OverviewSidebarOptions.test.tsx Show resolved Hide resolved
web/src/SidebarResources.tsx Outdated Show resolved Hide resolved
@landism landism merged commit f0795ec into master Feb 9, 2021
@landism landism deleted the mlandis/ch11050/alerts-on-top-toggle-visible-even-when-no branch February 9, 2021 20:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants