-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Make "participation not allowed" message accessible #4764
Merged
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
586e3ae
to
d772542
Compare
f8a6d69
to
bf6488c
Compare
d772542
to
4c15a1e
Compare
bf6488c
to
fe71073
Compare
4c15a1e
to
8ced8e1
Compare
fe71073
to
ffb17d2
Compare
8ced8e1
to
2959325
Compare
ffb17d2
to
de70aac
Compare
2959325
to
e181245
Compare
de70aac
to
ccd0e44
Compare
e181245
to
c56d902
Compare
ccd0e44
to
9f34acd
Compare
c56d902
to
a3292d2
Compare
9f34acd
to
dda3fb8
Compare
dda3fb8
to
969c7b8
Compare
e4297ce
to
feeded8
Compare
feeded8
to
2baf316
Compare
969c7b8
to
5a0a122
Compare
5a0a122
to
74820be
Compare
86c6af8
to
cf059dc
Compare
aed5bdb
to
4aae2ce
Compare
2e3db9a
to
aa2c551
Compare
This message is only shown in the `show` action, so it's shown at most once in the whole page, so it doesn't take up too much space and it isn't as overwhelming as if we were showing it on the index page, once per debate. We're only showing it when there are closed options, though, since there's already a message to sign in to comment when the question accepts open answers.
This way it's consistent with what we get when there are no comments. It only appears once on the page so it isn't overwhelming.
Not sure whether small or normal is better, but at least it's the same.
Hovering over the votes showed a "participation not allowed" message which was annoying when scrolling with the browser or simply moving the mouse around the page. Furthermore, it hid the information about the number of votes, links to show/collapse replies, ... We're planning to change the behavior of all the "participation not allowed" messages in order to show them on click instead of showing them on hover (just like it's done on touchscreens). In the case of comments, supports, however, there's very limited space in the part showing the number of supports for comments, so adding this message without breaking the layout is challenging. So, for now, we're simply redirecting unauthenticated users to the login page. If find an easy way to implement a better user interface in the future to display the "participation not allowed" message, we might change this behaviour.
4aae2ce
to
5a0a9aa
Compare
47cee46
to
fbe11cb
Compare
We were using the same logic six times regarding when we should show a "participation not allowed" message. Since we're going to change the current behavior, we're unifying the logic in one place so the changes will be easier.
Defining a behavior on hover means making it different for people using a keyboard or a touchscreen (most of the population, nowadays). In this case, we had an accessibility issue where the message wouldn't disappear once it appeared. That meant that, after tabbing through all the links and buttons in, for instance, the debates index, the page would be filled with "participation not allowed" messages, and in order to see the information about how many people have voted, reloading the page was required. For touchscreen users the behavior was similar to what we get on hover, although we've found some inconsistencies when trying to support several elements on the same page. We think in proposals it makes sense to hide the "support" button when users click on it, and the same applies to the buttonsto support and vote investment projects. However, we aren't hiding the buttons to agree/disagree with a debate in order to keep the information about the current number of people agreeing and disagreeing visible. Note we're removing some support spec methods because after these changes the duplication isn't as obvious as it was in the past.
fbe11cb
to
abec716
Compare
taitus
approved these changes
Feb 23, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
References
Objectives
Visual Changes
Before these changes
The message appears when hovering over the buttons to vote:
The message to sign in to comment appears when hovering over the "publish comment" button and the message to sign in to vote appears when hovering over the icons to agree and disagree with a comment:
before.mp4
After these changes
The message appears after clicking a button to vote:
The message to sign in to comment is displayed at all times and agreeing/disagreeing with comments redirects to the sign in page:
after.mp4