Skip to content

Fix for PR #5716 Move Logout button to end, and added name to CREDITS.md#5907

Closed
MananDhiman wants to merge 2 commits intoFreshRSS:edgefrom
MananDhiman:5716-header.phtml
Closed

Fix for PR #5716 Move Logout button to end, and added name to CREDITS.md#5907
MananDhiman wants to merge 2 commits intoFreshRSS:edgefrom
MananDhiman:5716-header.phtml

Conversation

@MananDhiman
Copy link
Copy Markdown

@MananDhiman MananDhiman commented Nov 26, 2023 •
edited
Loading

Closes #5716

How to test the feature manually:

  1. After login, open nav menu using gear icon in top right and check at different resolutions and devices

Pull request checklist:

  • code manually tested

Preview
image

@Alkarex Alkarex added this to the 1.23.0 milestone Nov 26, 2023
@Alkarex Alkarex added the UI 🎨 User Interfaces label Nov 26, 2023
Alkarex
Alkarex reviewed Nov 26, 2023
View reviewed changes
CREDITS.md
* [yzqzss|一座桥在水上](https://github.com/yzqzss): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:yzqzss), [Web](https://blog.othing.xyz/)
* [Zhaofeng Li](https://github.com/zhaofengli): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:zhaofengli), [Web](https://zhaofeng.li/)
* [Zhiyuan Zheng](https://github.com/zhzy0077): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:zhzy0077)
* [Manan Dhiman](https://github.com/manandhiman): [contributions](https://github.com/FreshRSS/FreshRSS/pulls?q=is:pr+author:manandhiman), [Web](https://manandhiman.com)
Copy link
Copy Markdown
Member

@Alkarex Alkarex Nov 26, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please keep alphabetic order ;-)

@math-GH
Copy link
Copy Markdown
Contributor

math-GH commented Nov 26, 2023 •
edited
Loading

I am against this change. Sorry for that.
Let me explain.
I made the PR #3880 2 years ago that moved the logout link from bottom to top. Please see the reason there.

IMHO it is not a good UX, when the logout button is not well placed. The logout is connected to the user account, because the current user will be logged out (semantic logic).
If the user wants to logout, have the mouse way in mind: Before the PR (logout button is on the top of dropdown list): User clikc on the cog button and moves down the mouse a bit to click the logout button (about 50px). This PR makes the way much longer: The mouse has to move from top of the window to the bottom of the list (about 600px). If the user use it one his/her mobile device he/she needs (maybe) to scroll the list to reach logout button.

To be honest: I do not understand the issue why the logout button is a problem on the top of list next to the user account section.

Suggestion: If you have troubles with the logout button on the top: An extension could easily improve it for you.

Strategic discussion: I would like to improve the menu in general. If there is an interest I could make a draft that visualize some ideas.

@Alkarex
Copy link
Copy Markdown
Member

Alkarex commented Nov 26, 2023

I personally do not mind one way or the other.
In any case, independently of the default, we could suggest a CustomJS rule (maybe doable with CustomCSS and an order rule) for people of have strong feelings one way or the other

@Alkarex
Copy link
Copy Markdown
Member

Alkarex commented Nov 26, 2023

I would like to improve the menu in general. If there is an interest I could make a draft that visualize some ideas.

👍🏻

@Alkarex
Copy link
Copy Markdown
Member

Alkarex commented Nov 26, 2023

@MananDhiman Sorry for the outcome, but your PR was just fine, so now you know how to do it for another time :-)

Examples of issues, which might not be too hard to implement:

@MananDhiman
Copy link
Copy Markdown
Author

MananDhiman commented Nov 27, 2023 •
edited
Loading

Perfectly fine, I'm just here to learn and practice. Always a next time.

Do I now close the pull request or something?

@Alkarex Alkarex closed this Nov 27, 2023
@math-GH math-GH added the UX User experience label Nov 27, 2023
@math-GH
Copy link
Copy Markdown
Contributor

math-GH commented Dec 22, 2023

As suggested: Here is my draft that includes a better logout button position: #5966

@aledeg aledeg mentioned this pull request Sep 30, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@Alkarex Alkarex Alkarex left review comments

Assignees

No one assigned

Labels

UI 🎨 User Interfaces UX User experience

Projects

None yet

Milestone

1.23.0

Development

Successfully merging this pull request may close these issues.

[Feature] Move logout button to the end of list

3 participants

@MananDhiman @math-GH @Alkarex