-
-
Notifications
You must be signed in to change notification settings - Fork 283
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
fix background color style #2326
Conversation
if the inset was set, background was #f7f7f7, I think the preview mode follows the dev |
I believe the preview is correct, it's the canvas that overlays a slight transparent grey when nothing is selected. The idea is to emphasize something is selected by slightly dimming the rest. It gives the illusion of a grey background. We can start by disabling this overlay when nothing is selected. If this turns out to still be confusing we will disable it altogether and rely purely on the blue box to indicate selection. |
Signed-off-by: Jerry_wu <[email protected]>
Signed-off-by: Jerry_wu <[email protected]>
Signed-off-by: Jerry_wu <[email protected]>
Signed-off-by: Jerry_wu <[email protected]>
If we don't want a background at all, you may as well remove this whole component |
actually, I removed the whole component(PinholeOverlay.tsx) but the |
Yes, you may have to move the |
@Janpot |
I added a test for status switching |
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.
If the PinholeOverlay does have utility and the only issue was the darkening of the UI I think just removing that darkening should do.
And thanks for adding a test for this! Adding some suggestions for the test:
Signed-off-by: Jerry_wu <[email protected]>
@apedroferreira |
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.
Almost there, just a few more important changes in the test!
Also just a nitpick, but feel free to use more vertical space in the test to separate bits of logic, it can really help improve the readability!
With my comments above, here's how I would vertically space the test, order the variables and change variable names to improve readability and make the test as easy to understand as possible:
|
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.
Awesome, thanks a lot!
|
fix #2301