-
Notifications
You must be signed in to change notification settings - Fork 16
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
[#7864] comments and ratings: small changes following mB redesign #1669
Conversation
4b66fe1
to
68d89e8
Compare
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.
looks good so far, some small changes and a question about terminology
@@ -7,7 +7,9 @@ import config from '../../../static/config' | |||
|
|||
const translations = { | |||
upvote: django.gettext('Click to vote up'), | |||
downvote: django.gettext('Click to vote down') | |||
downvote: django.gettext('Click to vote down'), | |||
likes: django.gettext('Likes'), |
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.
when read out by a sr this will say "Click to vote up" but then say "x likes" which doesn't feel very coherent. Do we need a mechanism (flag/prop) to either use a vote terminology (aplus) or a like terminology (mB) ?
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.
i would suggest this needs to be unified across platforms, otherwise we are adding lots of custom flags and edge cases for the real small stuff when the component has not been designed with this in mind. i would also argue we need to move on with merging this story as not to increase the scope even more, so i can create an issue.
@TeaJayyy @CarolingerSeilchenspringer what do we do? its not so simple as the terminologies are indeed different. On mB we need the text Likes
/Dislikes
to show, whereas on aplus we dont show anything (which forces us to overengineer things for screen readers and have the hidden message Click to vote
). Can we just have both platforms show text and then we can show and hide it visually with css on the respective forks? An idea on how not to add more logic.
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.
added issue: #1673
9d359b7
to
0f587b4
Compare
@hom3mad3 the npm tests are failing. can you have a look? once resolved, I am happy to merge this PR, and following with the ones in aplus and mB right after. |
added an issue for the unaddressed concern: #1673
0f587b4
to
cbc9ad7
Compare
dependencies:
liqd/a4-meinberlin#5686
liqd/adhocracy-plus#2747
Tasks