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

Show configured rate limit in the GUI #5320

Closed
uok opened this Issue Nov 12, 2018 · 15 comments

Comments

Projects
None yet
5 participants
@uok
Copy link
Contributor

uok commented Nov 12, 2018

Currently a rate limit (global or per device) is nowhere displayed in the GUI. To find out which device is rate limited you need to edit one device after the other to check the settings.
Here are some ideas how to display it:

Text note
limit-text
Icon
limit-icon
Text line
limit-line
Text info
limit-muted

If no rate limit is set the icon/texts are hidden.
My preference is icon version

@calmh

This comment has been minimized.

Copy link
Member

calmh commented Nov 12, 2018

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

AudriusButkevicius commented Nov 12, 2018

Agree, we should just have the grey text beneath displaying the limits, both global (on "self" device) and per device.

@uok

This comment has been minimized.

Copy link
Contributor

uok commented Nov 13, 2018

ok, I added grey text version

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

AudriusButkevicius commented Nov 13, 2018

The Capalised Sentences look weird, but otherwise looks ok.

@uok

This comment has been minimized.

Copy link
Contributor

uok commented Nov 13, 2018

I copied the text from settings and also the other labels have the same style.
If the writing is changed I guess it needs to be translated again?
We could just go with a general "Rate Limit: 123 KiB/s" in both cases, as it should be clear what it means. But this would need translation.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

AudriusButkevicius commented Nov 13, 2018

I guess if we are reusing an existing string is has to stay this way. Anyways, I'll let someone else chip in.

@calmh

This comment has been minimized.

Copy link
Member

calmh commented Nov 13, 2018

Existing translations are good enough imho

@calmh calmh closed this Nov 13, 2018

@calmh

This comment has been minimized.

Copy link
Member

calmh commented Nov 13, 2018

Woops

@clintmoyer

This comment has been minimized.

Copy link

clintmoyer commented Nov 13, 2018

I submitted a change that details the rate limit in a tooltip.

#5327

@uok

This comment has been minimized.

Copy link
Contributor

uok commented Nov 14, 2018

@clintmoyer thanks for contributing, but a tooltip is not needed as it does not really help to show which device has a rate limit.

uok added a commit to uok/syncthing that referenced this issue Nov 14, 2018

gui: Display rate limit (fixes syncthing#5320)
Display rate limit for own device and connected devices
@uok

This comment has been minimized.

Copy link
Contributor

uok commented Nov 14, 2018

Please check PR #5328

uok added a commit to uok/syncthing that referenced this issue Nov 14, 2018

gui: Display rate limit (fixes syncthing#5320)
Display rate limit for own device and connected devices
@clintmoyer

This comment has been minimized.

Copy link

clintmoyer commented Nov 15, 2018

My initial pull request included a tooltip for every device, as well as the global configuration. It seems like the prototype suggested here introduces a new "sub-row" of text to each configuration. Is that what we want? No other widget in the GUI has that offset right now.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

AudriusButkevicius commented Nov 15, 2018

Ignore patterns do that by adding text under the file counts.

@canton7

This comment has been minimized.

Copy link
Member

canton7 commented Nov 15, 2018

Ignores are called out that way because the presence of ignores helps explain information that might otherwise be confusing. This same applies here: the presence of a rate limit helps explain a low transfer rate that might otherwise be confusing.

Therefore I think it's right that this is called out in grey text underneath the transfer rate, rather than being hidden in a tooltip.

@AudriusButkevicius

This comment has been minimized.

Copy link
Member

AudriusButkevicius commented Nov 15, 2018

Agree

@calmh calmh closed this in 513d3bc Nov 22, 2018

fragtion added a commit to fragtion/syncthing that referenced this issue Nov 27, 2018

gui: Display rate limit (fixes syncthing#5320) (syncthing#5328)
Display rate limit for own device and connected devices

fragtion added a commit to fragtion/syncthing that referenced this issue Nov 28, 2018

gui: Display rate limit (fixes syncthing#5320) (syncthing#5328)
Display rate limit for own device and connected devices

fragtion added a commit to fragtion/syncthing that referenced this issue Nov 28, 2018

gui: Display rate limit (fixes syncthing#5320) (syncthing#5328)
Display rate limit for own device and connected devices

@calmh calmh added this to the v0.14.54 milestone Dec 5, 2018

@calmh calmh changed the title gui: Display rate limit Show configured rate limit in the GUI Dec 11, 2018

@calmh calmh added the enhancement label Dec 11, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment