-
-
Notifications
You must be signed in to change notification settings - Fork 2.4k
Rework the user actions menu #17942
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
base: master
Are you sure you want to change the base?
Rework the user actions menu #17942
Conversation
…ts in a dropdown window
I have no stats as well, but possible the "Challenge," "Message," "Block," and "Follow" buttons are the most needed as that original petition post mentioned. |
good stuff - it's an improvement |
not sure what is the point of returning an empty menu on error
4622c17
to
f026bd6
Compare
relation: Option[Relation], | ||
followable: Boolean, | ||
blocked: Boolean | ||
)(using ctx: Context) = |
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.
this entire function seems to duplicate the existing actions
function, trying to do the same thing but in a different format. It already has some discrepancies and it will only get harder to maintain.
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 was wondering whether to rely on the same function or rewrite one entirely that would return an equivalent for the user actions menu. I settled for another function to avoid impacting other functions relying on the existing actions
function, but I agree it's adding more complexity.
Do you have any suggestion on how to proceed on this matter?
) | ||
).flatten | ||
).flatten | ||
|
||
def actions( |
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.
this function has to be deleted to justify adding actionsMenu
. Probably with Menu
being able to write as HTML additionally to JSON.
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.
Oh, just read this comment after the first one. So that's your suggestion then, only keeping one function that would return a Menu object (which could potentially be renamed actions
) and have the Menu class be able to serialize either in JSON or HTML.
two small nits - could we shorten "Challenge to a game" to "Challenge" and "Compose message" to "Message" or something? it could get us another button in default layouts and i don't think any nuance is lost. |
About a year ago, I wrote the PR #15260 addressing the issue #13991 regarding the user actions menu on mobile.
The issue reported initially had to do with the user action buttons not displaying any labels which would hide their underlying behaviour to mobile users as touchscreen devices aren't able to hover on components.
The other issue I saw with the original menu was that even on desktop, one had to hover on a button to get an idea on what the behaviour would be when clicking on it, which appeared like bad UX design. The PR then addressed both of these issues by displaying the user actions with labels instead of just the icons. But it did so in a dropdown menu as the page was lacking space.
When this PR was merged and live, @gbtami relayed that some users complained about the new design being worse than before on desktop as it wasn't making use of this extra space and was hiding every user actions under a dropdown menu, which was a fair point.
@gbtami then suggested another design that was compromising by displaying the most frequent actions straight away and hiding the rest of the actions that couldn't be shown in a dropdown menu. I really liked the idea and then offered to develop it on Lichess' Discord. But I never took the time to actually dive in it, but I did now.
The key features of this new menu are:
Now I had to make choices as to what buttons are more relevant and thus should be placed first in the menu to get a better chance of being displayed straight away. I also favoured actions with shorter labels to fit more buttons. But I don't have the statistics to know what actions are actually the most commonly used among Lichess uers.
Here's a gif showing how it looks:
Edit: I've just realized that there's an issue with the
Impersonate
menu. I didn't test that one. I'll see how to fix that.