-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Decrease scrim when in a modal overlay #27054
Conversation
My concern would be that this would introduce some accessibility issues. The purpose of the scrim is specifically to hide any changes in the background. Lightening the scrim means you are relying on someone seeing what is happening to understand the change - which kind of goes against the whole idea of a modal focusing the user? It looks like this was part of the reason @kjellr darkened it in #15974 at least. If the problem is that the settings aren't clear, adjusting the modal content with better wording or even adding an example might be a better approach? Maybe you could try having a small example below the setting that changes with it? |
I don't mind it at all. The fact that the popop immediately takes attention feels focus grabbing enough (not to mention the literal focus grabbing). However I believe we tried this in the past, and decided to keep the scrim the same as it is for the classic WordPress lightboxes such as the media library: I don't feel particularly strongly that the two need to stay in perfect lockstep, in fact longer term I'd prefer to replace the latter with the former. But sharing as context. |
Thanks for the feedback so far everyone. @brentswisher, I totally understand your concerns, however I think I can perhaps respond to a few of them with some context of how I got to this solution. I totally agree that iterating text and being as clear as possible should be the first step, there are cases where words just aren't enough. As explained, I have tried adding things like examples, but it ended up feeling like they were clickable or even increasing distraction. Now, this could be a case of needing to iterate what was showing, something I am open to exploring outside this PR. This is where I would like to point out something, this suggestion isn't about removing the scrim, the idea is to adjust the effect. Initially it was brought in for a few reasons, there was inconsistency in there being white and dark scrims and there was also in some places none. What this does is reduce the strength. If the idea is to not see the background, I would argue when something is changing like a setting it could be more cognitive dissonance to see a slight shadow change than actual change. Of course, this could do with testing and feedback. All of this said, if the feedback is to not progress here I am very open to that, I just wanted to be clear about why this exploration was being suggested. It really just is at this point an experiment so thank you both for joining me in early feedback. @jasmussen your point about what we can or can't control in classic is making me pause here, if that is the case then I'm cautious about continuing and would like to review how the flow feels or how deep the fix would be. That feels like a next step for me! |
Here is what it looks like going between both screens, I can confirm there is no way to change this outside of a Trac patch. @jasmussen and anyone else, would love your feedback on how much of a visual hitch this is. My own feeling is if I'm honest I could go either way on this. It was an experiment and if opinion is to leave to one side for now I am quite fine with that. |
I like it, I'd be happy to try it. We could always position this as a "lighter" alternative to the bulkier media dialog modals. |
@jasmussen that's great. I do think because one is near-full and the other is in middle you almost get away without seeing it. I did follow quite a flow not many are going to also going between both of them. Once the tests have passed I will get this merged and we can take from there. Thanks for input. As with anything we try in the editor this can of course be iterated and I like idea of seeing here before proposing wider. |
b95b3d7
to
96f8994
Compare
Size Change: 0 B Total Size: 1.2 MB ℹ️ View Unchanged
|
I'm really not sure why these tests are failing 🤔 . It looks like there's inconsistencies with the test results from the commits on There should be no reason why this CSS change would break E2E Admin 1. Based on this, I think it's okay to merge 👍 |
Thanks for second opinion @ItsJonQ - let's merge and I am very happy to undo that if feedback arises. |
This is a little PR that certainly needs feedback. The problem it is designed to solve is specifically when you change a setting on preferences, it's hard to see what the change was. You can see that here:
By increasing the transparency, you are able to more easily see the change in effect as it happens:
This is useful when words are hard to describe what is going on. At times, simply seeing in the interface is a great help. I did explore having partial scrims and even none, they all resulted in a more negative experience or visual hitch.
Feedback
There are a few specific areas along with more general feedback that would be good to get input on whether there was a reason this was set to the opacity it was and by unsetting there is an impact.