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

Make user status dialog look in line with the rest of the desktop client tray and Nextcloud #4624

Merged
merged 1 commit into from
Jun 24, 2022

Conversation

claucambra
Copy link
Collaborator

Currently, the components of the user status selection dialog are not styled at all:

Screenshot 2022-06-08 at 14 45 09

This means the dialog doesn't follow the system theme and it also looks different from the server's user status selection dialog.

This PR changes that so that the elements of the dialog match the system theme as well as the rest of the desktop client tray and the user status selection dialog of the server:

Screenshot 2022-06-08 at 15 00 14

Screenshot 2022-06-08 at 15 00 26

@jancborchardt
Copy link
Member

Awesome @claucambra! :) Just 3 suggestions:

  • The text and sublines of the Do not disturb and Invisible status are a bit far apart, maybe half that
  • The border of the status input field looks a bit dark, or is that standard
  • For better alignment of the status suggestions, the emojis and text should have a bit more whitespace inbetween, so that the emojis line up with the emoji picker above, and the text left-aligns with the input field text "What is your status?"

@claucambra claucambra force-pushed the feature/userstatusdialog-design branch from fa6d6fd to 0c3cf72 Compare June 8, 2022 15:04
@claucambra
Copy link
Collaborator Author

Awesome @claucambra! :) Just 3 suggestions:

  • The text and sublines of the Do not disturb and Invisible status are a bit far apart, maybe half that
  • The border of the status input field looks a bit dark, or is that standard
  • For better alignment of the status suggestions, the emojis and text should have a bit more whitespace inbetween, so that the emojis line up with the emoji picker above, and the text left-aligns with the input field text "What is your status?"

Hey @jancborchardt, some changes to fit your suggestions:

Screenshot 2022-06-08 at 17 03 29

@claucambra claucambra force-pushed the feature/userstatusdialog-design branch from 0c3cf72 to 81d232b Compare June 9, 2022 09:23
@splitt3r
Copy link

splitt3r commented Jun 9, 2022

Great work! This looks pretty neet.

resources.qrc Outdated Show resolved Hide resolved
src/gui/BasicComboBox.qml Outdated Show resolved Hide resolved
src/gui/PredefinedStatusButton.qml Outdated Show resolved Hide resolved
src/gui/UserStatusSelector.qml Show resolved Hide resolved
src/gui/UserStatusSelector.qml Show resolved Hide resolved
@jancborchardt
Copy link
Member

@claucambra the text just needs to move a bit to the right, see this illustration :)
Also little detail: Since we are trying to get rid of the word "user" in the interface, let’s just title the dialog "Set status".

Desktop set status

@claucambra
Copy link
Collaborator Author

@claucambra the text just needs to move a bit to the right, see this illustration :) Also little detail: Since we are trying to get rid of the word "user" in the interface, let’s just title the dialog "Set status".

Desktop set status

Forgive me but I can't really see the issue here, everything seems aligned? Or would you like the text in general to have more padding?

@codecov
Copy link

codecov bot commented Jun 14, 2022

Codecov Report

Merging #4624 (3501640) into master (f27aa4f) will increase coverage by 0.04%.
The diff coverage is n/a.

❗ Current head 3501640 differs from pull request most recent head 50b4282. Consider uploading reports for the commit 50b4282 to get more accurate results

@@            Coverage Diff             @@
##           master    #4624      +/-   ##
==========================================
+ Coverage   56.52%   56.56%   +0.04%     
==========================================
  Files         138      138              
  Lines       17071    17071              
==========================================
+ Hits         9649     9656       +7     
+ Misses       7422     7415       -7     
Impacted Files Coverage Δ
src/libsync/propagatedownload.cpp 65.18% <0.00%> (+1.03%) ⬆️

@mgallien
Copy link
Collaborator

@claucambra please do no forget to clean history as one more time I forgot to ask this before approving

@claucambra claucambra requested a review from nimishavijay June 15, 2022 10:29
@jancborchardt
Copy link
Member

@claucambra yes sorry – you need to compare my screenshot (with the red lines) with the one you posted before that. :) I moved the text slightly to the right in mine.

@claucambra
Copy link
Collaborator Author

claucambra commented Jun 17, 2022

@claucambra yes sorry – you need to compare my screenshot (with the red lines) with the one you posted before that. :) I moved the text slightly to the right in mine.

Ah! Sorry 🤦‍♂️ here is an updated version:

Screenshot 2022-06-17 at 11 31 39

@nimishavijay
Copy link
Member

nimishavijay commented Jun 23, 2022

Looks really nice now! 🚀🚀🚀🚀 :)

Edit: I thought it needed some padding on the outside, but on second thought it looks great already :)

@mgallien
Copy link
Collaborator

Looks really nice now! rocketrocketrocketrocket :)

Edit: I thought it needed some padding on the outside, but on second thought it looks great already :)

@nimishavijay could you add your approval such that we can merge it ?

@mgallien
Copy link
Collaborator

@claucambra there is a conflict, can you have a look ?

@claucambra claucambra force-pushed the feature/userstatusdialog-design branch from 5b5b639 to 46f2bb5 Compare June 24, 2022 12:02
@claucambra
Copy link
Collaborator Author

@claucambra there is a conflict, can you have a look ?

Rebased and solved :)

@mgallien mgallien force-pushed the feature/userstatusdialog-design branch from 46f2bb5 to 3501640 Compare June 24, 2022 13:20
@mgallien
Copy link
Collaborator

@claucambra there is again a conflict, can you have a look ?

@mgallien
Copy link
Collaborator

@claucambra please clean history before merging

@claucambra claucambra force-pushed the feature/userstatusdialog-design branch from 3501640 to 50b4282 Compare June 24, 2022 13:53
@claucambra
Copy link
Collaborator Author

@mgallien squashed and rebased

Signed-off-by: Claudio Cambra <claudio.cambra@gmail.com>
@claucambra claucambra force-pushed the feature/userstatusdialog-design branch from 50b4282 to ee3e0d3 Compare June 24, 2022 14:29
@claucambra claucambra merged commit 8f5624c into master Jun 24, 2022
@claucambra claucambra deleted the feature/userstatusdialog-design branch June 24, 2022 14:30
@nextcloud-desktop-bot
Copy link

AppImage file: Nextcloud-PR-4624-ee3e0d335182617b5c9140882ca725d48ccbe07e-x86_64.AppImage

To test this change/fix you can simply download above AppImage file and test it.

Please make sure to quit your existing Nextcloud app and backup your data.

@sonarcloud
Copy link

sonarcloud bot commented Jun 24, 2022

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
No Duplication information No Duplication information

@mgallien mgallien added this to the 3.6.0 milestone Jul 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants