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

[UX][A11Y] Visually indicate a View Display is disabled #4293

Open
laryn opened this issue Jan 24, 2020 · 14 comments
Open

[UX][A11Y] Visually indicate a View Display is disabled #4293

laryn opened this issue Jan 24, 2020 · 14 comments

Comments

@laryn
Copy link
Contributor

laryn commented Jan 24, 2020

The main content of the view display gets greyed out if it is disabled, but the main link to the display does not. I think it makes sense to carry that through to the display links at the top like this:

Screen Shot 2020-01-24 at 11 27 48 AM


PR: backdrop/backdrop#3049
Alt PR: backdrop/backdrop#3058

@indigoxela
Copy link
Member

Good catch and it's a minimal change.

@ghost
Copy link

ghost commented Jan 26, 2020

My only concern is that people may not realise they can click on the disabled view display to re-enable it...

@jenlampton jenlampton added this to the 1.15.1 milestone Jan 29, 2020
@jenlampton jenlampton changed the title Visually indicate a View Display is disabled [UX] Visually indicate a View Display is disabled Jan 29, 2020
@jenlampton
Copy link
Member

We also need to check that contrast for accessibility. We might want to do a ghost button instead.

@jenlampton
Copy link
Member

Yeah, there's not enough contrast between the button background and the text color:
Screen Shot 2020-01-28 at 5 25 01 PM

@jenlampton
Copy link
Member

Well, it looks the contrast on the rest of the disabled UI is not readable either... Should we fix both in this issue, or create another?
Screen Shot 2020-01-28 at 5 30 23 PM

@klonos
Copy link
Member

klonos commented Feb 2, 2020

How about indicating disabled displays by making the "This display is disabled" text a warning at the top of the page, instead of using opacity? That way, we don't have to worry about readability. Then we could be appending (disabled) in the display link as well.

Something like this for example?:

Screen Shot 2020-02-02 at 11 42 30 am

...and I think that we should also be indicating disabled displays in the main views listing page:

Screen Shot 2020-02-02 at 11 57 24 am

PR: backdrop/backdrop#3058

@olafgrabienski
Copy link

olafgrabienski commented Feb 2, 2020

How about indicating disabled displays by making the "This display is disabled" text a warning at the top of the page, instead of using opacity?

Not sure but an Interesting idea! If we choose this approach, I'd prefer a blue info message instead of a warning to not confuse it with the "All changes are stored temporarily" warning which appears as soon as you start to configure a view. In any case, for a disabled display it should be sufficient to just inform about the state.

@klonos
Copy link
Member

klonos commented Feb 3, 2020

Yup, it indeed feels better with the message being of type info instead of warning. Great suggestion @olafgrabienski 👍 (I've updated the PR)

@olafgrabienski
Copy link

Thanks for updating the PR, @klonos. As we have two PRs now, I've added the needs - more feedback label to the issue. I'm still not sure which approach to prefer. However, if we choose your PR, I'd suggest to display the name of a disabled display as 'line-through' (see screenshot mockup at bottom).

I've also tested @klonos' PR in the sandbox site. It works fine but when I saved a view, the info message was rendered twice. Speaking of the info message, I'd prefer to not show the info at the top but below the warning message, so that the info is placed directly above the Displays heading when there are other messages.

Screenshot mockup: strike-through for name of disabled display

screen-backdrop-pr-3058-b-linethrough

@klonos
Copy link
Member

klonos commented Feb 3, 2020

I'd suggest to display the name of a disabled display as 'line-through'

I'm neither hot nor cold re this; let's wait for some more feedback before I update the PR.

when I saved a view, the info message was rendered twice.

Ah yes, this does happen sometimes. Usually the solution is to add a "blank" backdrop_get_messages() before the actual backdrop_set_message(t('bla bla bla')). I'll test locally and update the PR if that indeed fixes the issue.

I'd prefer to not show the info at the top but below the warning message

Hmm 🤔 I don't think that you can control the order of messages thrown by backdrop_set_message() (although that'd be nice), but I have an idea re this issue: we can use the same approach as in #3330 (but we should really at some point implement #3329 in the Form API level). ...I've updated the PR to implement it that way. Here's current behavior:

Screen Shot 2020-02-03 at 11 07 29 pm

...and here's a screenshot from the updated PR (with the "dirty form" message):

Screen Shot 2020-02-03 at 11 10 42 pm

@ghost
Copy link

ghost commented Feb 3, 2020

Is the duplicated messages related to #4247? Maybe it's a broader issue...

@klonos
Copy link
Member

klonos commented Feb 3, 2020

Yes @BWPanda, it is. I've missed that issue, so thanks for pointing it out 👍

@laryn
Copy link
Contributor Author

laryn commented Feb 3, 2020

I like this (and agree we might as well fix the grey-out accessibility issue right in this issue). The info message is great, and adding "(disabled)" to the display name works for me. I don't have a strong opinion on whether to also add a lineout to the name.

@jenlampton jenlampton removed this from the 1.15.1 milestone Mar 19, 2020
@quicksketch quicksketch modified the milestones: 1.17.2, 1.17.3 Nov 8, 2020
@jenlampton jenlampton modified the milestones: 1.17.3, 1.17.4, 1.17.5 Nov 19, 2020
@jenlampton jenlampton modified the milestones: 1.17.5, 1.18.1, 1.18.2 Jan 14, 2021
@jenlampton jenlampton modified the milestones: 1.18.2, 1.18.3 Mar 24, 2021
@jenlampton jenlampton modified the milestones: 1.18.3, 1.18.14 Apr 21, 2021
@jenlampton jenlampton modified the milestones: 1.18.4, 1.19.1 May 16, 2021
@jenlampton jenlampton modified the milestones: 1.19.1, 1.19.2 May 26, 2021
@quicksketch quicksketch modified the milestones: 1.19.2, 1.19.3 Jul 21, 2021
@jenlampton jenlampton modified the milestones: 1.19.3, 1.19.4 Aug 12, 2021
@jenlampton jenlampton changed the title [UX] Visually indicate a View Display is disabled [UX][A11Y] Visually indicate a View Display is disabled Aug 16, 2021
@quicksketch quicksketch modified the milestones: 1.19.4, 1.20.1 Sep 15, 2021
@quicksketch quicksketch modified the milestones: 1.20.1, 1.20.2 Oct 11, 2021
@indigoxela
Copy link
Member

Seems like this issue has stalled back in early 2020. Changes have been requested, but never got addressed. Additionally, this PR contains loads of completely unrelated changes. Time to remove the milestone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants