Skip to content

Add a 'Sync now' button to the sync status header in the tray window #5018

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Oct 25, 2022
Merged

Add a 'Sync now' button to the sync status header in the tray window #5018

merged 3 commits into from
Oct 25, 2022

Conversation

claucambra
Copy link
Contributor

@claucambra claucambra commented Oct 7, 2022 •
edited
Loading

image

Closes #4877

Signed-off-by: Claudio Cambra claudio.cambra@gmail.com

@claucambra claucambra self-assigned this Oct 7, 2022
mgallien
mgallien reviewed Oct 12, 2022
View reviewed changes
Copy link
Collaborator

@mgallien mgallien left a comment

Choose a reason for hiding this comment

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

works well
will be very convenient
but as an user I would never discover this hidden gem
we should have visible on hover feedback

@claucambra
Copy link
Contributor Author

works well will be very convenient but as an user I would never discover this hidden gem we should have visible on hover feedback

Understood, will add a hover effect

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 064b753 to 6a542c1 Compare October 12, 2022 16:52
@claucambra
Copy link
Contributor Author

works well will be very convenient but as an user I would never discover this hidden gem we should have visible on hover feedback

Done, here's a video:

simplescreenrecorder-2022-10-12_18.51.32.mp4

@claucambra claucambra requested a review from mgallien October 12, 2022 16:53
@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 6a542c1 to 88a678b Compare October 12, 2022 17:00
@codecov
Copy link

codecov bot commented Oct 12, 2022 •
edited
Loading

Codecov Report

Merging #5018 (c0baf5c) into master (6db3361) will decrease coverage by 0.00%.
The diff coverage is 55.55%.

Additional details and impacted files 
@@            Coverage Diff             @@
##           master    #5018      +/-   ##
==========================================
- Coverage   57.22%   57.21%   -0.01%     
==========================================
  Files         138      138              
  Lines       17444    17441       -3     
==========================================
- Hits         9982     9979       -3     
  Misses       7462     7462
Impacted Files Coverage Δ
src/libsync/owncloudpropagator.h 68.42% <0.00%> (-6.02%) ⬇️
src/libsync/syncengine.h 50.00% <57.14%> (+6.25%) ⬆️
src/libsync/syncengine.cpp 84.16% <100.00%> (-0.08%) ⬇️
src/libsync/vfs/cfapi/cfapiwrapper.cpp 74.09% <0.00%> (-0.23%) ⬇️
src/libsync/propagatedownload.cpp 64.61% <0.00%> (+1.17%) ⬆️

@jancborchardt
Copy link
Member

Hm, after looking at the video, thinking about it again, with @mgallien’s feedback, and @Andreas-Kainz’s original issue at #4877, I now agree with the proposal to actually just add a button there. :D

Should be on the right, saying "Sync now", doesn’t need an icon. And then the whole line does not need a hover state, cause that actually looks a bit off.

The button can be text-only or have a simple border around it (with pill-style radius). What do you think @claucambra?

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 88a678b to ce858e4 Compare October 20, 2022 11:44
@claucambra
Copy link
Contributor Author

Hm, after looking at the video, thinking about it again, with @mgallien’s feedback, and @Andreas-Kainz’s original issue at #4877, I now agree with the proposal to actually just add a button there. :D

Should be on the right, saying "Sync now", doesn’t need an icon. And then the whole line does not need a hover state, cause that actually looks a bit off.

The button can be text-only or have a simple border around it (with pill-style radius). What do you think @claucambra?

Like this? :)

image

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from ce858e4 to 91e6bf4 Compare October 20, 2022 11:54
@claucambra claucambra changed the title Make the sync status clickable to force a sync (if a sync not already running) Add a 'Sync now' button to the sync status header in the tray window Oct 20, 2022
@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 91e6bf4 to 4669671 Compare October 24, 2022 11:48
@mgallien mgallien force-pushed the feature/force-sync-tray-button branch from 4669671 to 51e4b4f Compare October 24, 2022 16:37
@mgallien
Copy link
Collaborator

@jancborchardt @nimishavijay are you OK with the current state ?

jancborchardt
jancborchardt approved these changes Oct 25, 2022
View reviewed changes
Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Yeah, looks great @claucambra, straight and to the point. :)

Will then for sure take care of cases where people want to absolutely sync stuff right now (before they shut down the device e.g.).

@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 51e4b4f to 9a25699 Compare October 25, 2022 17:13
@claucambra
Create commonly accessible properties for current user colours in Sty…
1a6326e 
…le.qml

Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
@claucambra
Add forcefoldersync method to folder manager
94b45d6 
Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
@claucambra
Add a 'Sync now' button to the sync status header in the tray window
c0baf5c 
Signed-off-by: Claudio Cambra <claudio.cambra@nextcloud.com>
@claucambra claucambra force-pushed the feature/force-sync-tray-button branch from 9a25699 to c0baf5c Compare October 25, 2022 17:14
@sonarqubecloud
Copy link

SonarCloud Quality Gate failed.    Quality Gate failed

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

0.0% 0.0% Coverage
0.0% 0.0% Duplication

@nextcloud-desktop-bot
Copy link

AppImage file: nextcloud-PR-5018-c0baf5c93cdc31349fcb8c4dd339784936faa92b-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.

@claucambra claucambra merged commit 95898fd into master Oct 25, 2022
@claucambra claucambra deleted the feature/force-sync-tray-button branch October 25, 2022 19:33
@AndyXheli
Copy link

#4608

@mgallien mgallien added this to the 3.7.0 milestone Nov 8, 2022
@allexzander
Copy link
Contributor

/backport to stable-3.6

@allexzander allexzander modified the milestones: 3.7.0, 3.6.3 Dec 6, 2022
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Dec 6, 2022
Closed
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mgallien mgallien mgallien approved these changes

@jancborchardt jancborchardt jancborchardt approved these changes

@camilasan camilasan Awaiting requested review from camilasan camilasan is a code owner

@allexzander allexzander Awaiting requested review from allexzander

@nimishavijay nimishavijay Awaiting requested review from nimishavijay

Assignees

@claucambra claucambra

Labels
None yet
Projects
None yet
Milestone
3.6.3
Development

Successfully merging this pull request may close these issues.

Add start sync button
6 participants
@claucambra @jancborchardt @mgallien @nextcloud-desktop-bot @AndyXheli @allexzander