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

webui: fixed width for speed info #6739

Merged
merged 3 commits into from
May 25, 2024
Merged

Conversation

tearfur
Copy link
Member

@tearfur tearfur commented Mar 26, 2024

Implement my suggestion at #6577 (comment).

Fix the width of the upload speed info, so that the download speed info will not move around in response to upload speed changes.

Before After
before after-2

Edit: Moved the arrows to the right of the text as suggested by #6739 (comment).

previous iteration

after

@tearfur tearfur added scope:web needs UI review This PRs has UI changes that need review type:ui notes:none Should not be listed in release notes labels Mar 26, 2024
@tearfur
Copy link
Member Author

tearfur commented Mar 26, 2024

CC @tessus, see what you think of this.

@tessus
Copy link
Contributor

tessus commented Mar 26, 2024

I am sorry, but I don't like it.

It makes me nervous just by looking at it. But we'll never agree on this I am afraid. It's just a personal preference. Apart from that it looks a bit unprofessional.

Imagine top or any other monitoring tool (btm, btop, ...), where the lines are not using columns but just print out data and every line is moving around on its own. Who the heck would be able to parse any info in that mess?

I really appreciate you trying to find a compromise though.

@tearfur tearfur force-pushed the webui-speed-label branch 2 times, most recently from b7faed9 to ff892e4 Compare March 26, 2024 04:00
@tearfur
Copy link
Member Author

tearfur commented Mar 26, 2024

Well, that's a shame.

I think this is a step closer to agreement though. If others agree, then I think this should be merged.

@Pentaphon
Copy link

Fix the width of the upload speed info, so that the download speed info will not move around in response to upload speed changes.

Absolutely an improvement. Less movement = less distractions. A definite merge for me.

Copy link
Member

@nevack nevack left a comment

Choose a reason for hiding this comment

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

I think this is a step closer to agreement though.

Yep, this is an improvement, and anybody can reiterate a better-looking solution later.

@tearfur tearfur removed the needs UI review This PRs has UI changes that need review label Mar 26, 2024
@Coeur Coeur added the needs UI review This PRs has UI changes that need review label Mar 26, 2024
@Coeur
Copy link
Collaborator

Coeur commented Mar 26, 2024

So, I'm totally aware that different systems do not need to share the same UI.
But as I wrote in the previous PR, we had this on the macOS app:
Capture d’écran 2024-03-23 à 20 19 25

And that change is introducing yet a third mechanism of displaying the up/down labels. So we'll have three ways of showing those arrows in Transmission (across two apps). Is there a way to find some guidance on internet regarding the "recommended" way to do this? Like looking at some big references of the web, and see how they do it?

The way you did it, Tearfur, could be "easily" transformed into fixed arrows by putting the arrows on the right of the values instead of the left of them, keeping the same total width. So I believe that your moving arrows isn't the ideal solution.

@tearfur
Copy link
Member Author

tearfur commented Mar 27, 2024

Even if it is not consitent, is that a problem if the users doesn't get bothered by it?

Edit: Just saw your suggestion of moving the arrows to the right. I'll make a screen recording of that, then we can see how it looks like.

Edit 2: @Coeur I think this is completely fine, wdyt?

after-2

@tessus
Copy link
Contributor

tessus commented Mar 27, 2024

I still don't understand what the issue is with my original fix. What is it that bothers you so much that you want to implement it that the data is moving around?

tr2

@tearfur
Copy link
Member Author

tearfur commented Mar 27, 2024

I still don't understand what the issue is with my original fix.

What bothers me is that the up arrow is closer to the download speed text than the upload speed text most of the time. I get it that you can still tell which is which by their order of appearance, but I don't want to replace a flaw with another flaw.

What is it that bothers you so much that you want to implement it that the data is moving around?

The data is not moving around in my implementation though...? Just the up/down arrows are moving.

I've also just posted another alternative that puts the arrow to the right of the text, so nothing moves now.

@tessus
Copy link
Contributor

tessus commented Mar 27, 2024

I've also just posted another alternative that puts the arrow to the right of the text, so nothing moves now.

Now this is something I can agree with. 👍 🎉 ❤️

@tearfur
Copy link
Member Author

tearfur commented Mar 27, 2024

Just committed the change to move arrows to the right.

@Coeur Coeur removed the needs UI review This PRs has UI changes that need review label Mar 27, 2024
@Coeur Coeur requested a review from nevack March 27, 2024 05:37
@Rukario
Copy link
Contributor

Rukario commented Mar 29, 2024

Not sure how I feel about this. It doesn't change that often to cause for distraction. It's short string that takes 5 seconds to refresh, the chance of digits changing is 1 out of 10, so gaps is more of a distraction :)

I MAY be ok with arrows switched to another side for less movements required to them.

Just something I noticed, I love your attractive screenshots showcasing your fixes, but the localization differences are providing different people between "s" and "sec", it can cause issue for those with "sec" and width appearing too wide for those with "s".

Please port the fix from the main branch for 4.0.x that has the issue, instead of creating another fix with preferences that will not be satisfied by everyone.

@tessus
Copy link
Contributor

tessus commented Mar 29, 2024

I am not sure who you are addressing. I am using en_US.UTF8 on all my machines. I haven't done any changes to the settings, so I have no idea why it is showing sec and not s.
This inconsistency is an issue to be discussed with the devs.

I am not doing anything to my PR for the 4.0.x branch. My fix has been there for months. I stopped looking at whether it's been merged and I don't care anymore. This project seems a bit weird with merging, release cadence, ...
I fix my local copy and I'm done. If my fix is no longer ok, my PR should be closed and somebody else can create a new PR. I guess it will take a few more months before it's merged anyway.

@tearfur
Copy link
Member Author

tearfur commented Apr 5, 2024

@Rukario Do you have a better feel for how this should be done now?

but the localization differences are providing different people between "s" and "sec", it can cause issue for those with "sec" and width appearing too wide for those with "s".

I wonder if there is anything we can do about this that is pure CSS... I am reluctant to use JavaScript for this, makes the styling code harder to trace. Suggestions?

@killemov
Copy link

killemov commented Apr 17, 2024

@tearfur The unit is "s" so localizations that use "sec" or anything else should be replaced.

In Shift I put the Unicode arrows next to each other in the title bar:
downloadupload

Also the jumping around of the speed indication numbers is less of an issue when the update interval is longer than a couple of seconds.

@Coeur
Copy link
Collaborator

Coeur commented Apr 21, 2024

@tearfur The unit is "s" so localizations that use "sec" or anything else should be replaced.

Our code is:

const fmt_kBps = new Intl.NumberFormat(current_locale, {
maximumFractionDigits: 2,
style: 'unit',
unit: 'kilobyte-per-second',
});
const fmt_MBps = new Intl.NumberFormat(current_locale, {
maximumFractionDigits: 2,
style: 'unit',
unit: 'megabyte-per-second',
});

So when it displays kB/sec, it's possibly a bug in that Javascript support itself on that operating system? Specs are at:
https://tc39.es/ecma402/#numberformat-objects. It mentions unitDisplay as extra option which could improve the rendering eventually by setting it to "narrow"? [edit] It's caused by locale being "en-CA".

In Shift I put the Unicode arrows next to each other in the title bar: downloadupload

That's nice too. But I'd prefer to stick with current approach in present PR for now.

Also the jumping around of the speed indication numbers is less of an issue when the update interval is longer than a couple of seconds.

I'd prefer to stick to 1 second refresh for a unit which is in extenso "per second".

@killemov
Copy link

So when it displays kB/sec, it's possibly a bug in that Javascript support itself on that operating system? Specs are at:
https://tc39.es/ecma402/#numberformat-objects. It mentions unitDisplay as extra option which could improve the rendering eventually by setting it to "narrow"?

Not a bug, but an inconsistency. Why only use the "short" version of part of the unit? So it should either be "kB/s" or "kilobyte/second". But I guess "narrow" would be a good start for some experimentation because I can't find what that specifies exactly. (Vendor specific maybe?)
BTW, only since I began torrenting "kilobyte/second" became common for me. Before that network speeds were known in "kilobit/second". And don't even get me started about when hard drive vendors started the 1024 vs. 1000 thing.

@tessus
Copy link
Contributor

tessus commented Apr 21, 2024

As an info, I am running the transmission daemon + webui on Fedora 39 Server with LANG=en_US.UTF-8.

I am using Firefox 125.0.1 (64-bit) on macOS 14.4.1 with the following Language & Region settings:

image

Please note that the browser uses English (US) as the language. I also tried other browsers, like Safari and Chrome.

The speed is displayed as xx/sec and I have no idea how to change it to xx/s.

I am only posting this for reference, because I noticed that I didn't describe my environment in my previous comment.

@Coeur
Copy link
Collaborator

Coeur commented Apr 22, 2024

Thank you tessus. I reproduced the sec display with "en-CA" locale, and it's not possible to fix it with unitDisplay: "narrow" (which will only remove the space between the value and the unit). Maybe it's correct behavior for Canada.

@tessus
Copy link
Contributor

tessus commented Apr 22, 2024

Hmm, I wouldn't know why Canadian locale should be different in that regard.
Additionally, yes, Canada uses the metric system (in most cases - I could write a very amusing article about that), but date/time/start_of_week are all a bit fucked up.

We use Sunday as the start of the week (which makes no sense unless you are in the Middle East), we use the 12h and the 24h clock, and we use way too many date formats. There is no proper standard in Canada for date and time. Which is why I always set them to YYYY-MM-DD/24h/Monday no matter which locale or region I set.

In my terminal on macOS my locale is en_US.UTF-8. It is only set to Canada in the Region pane in System Settings.

So now I tried setting the Region to United States and Safari shows xx/s. Which makes this even weirder because my Firefox browser actually uses the US locale, but still shows xx/sec, unless I change it in the System Settings of macOS.

I have always found localization extremely annoying. Why the heck does the world not use unambigious formats and be done with it. Anyway, I gave up on trying to understand or discuss this. I just don't care anymore and I have accepted for many years now that IT failed in this area big time and that this will never truly be fixed - ever.

@killemov
Copy link

@tessus That is what the ISO formats are for. Some insight for (at least) American readers here: The Metric Maven

@tessus
Copy link
Contributor

tessus commented Apr 22, 2024

That is what the ISO formats are for.

Hahaha. I haven't laughed that much and hard in a while. Thank you for that. Yes, I know that ISO is supposed to solve the ambiguity issue.

But products in Canada use e.g. the following formats for expiry date: MM/YY YY/MM DD/MM/YY YY/DD/MM MM/DD/YY and whatnot. if you are lucky they use MMM. Anyway, nobody gives a damn about ISO. I've been in IT for over 30 years. SW uses all kinds of formats, and if you are lucky the format is explained in some docs - but mostly it is not.

I lived and spent time in many parts of the world, Canada is not the exception, when it comes to ignorance of ISO.
Don't get me started with the US. Although at least they are mostly consistent with the MM/DD/YY format on government forms.

Unless ISO is used worldwide without exception, it is useless. I use ISO (or close to ISO w/o ambiguity), but that's just me. All companies I worked for ignored my plea to use ISO.

states/provinces in North America can't even get rid of this idiotic daylight savings time. Same shit is happening in Europe. They've been talking for years and years. What happened? Nothing. Anyway, my rant is way off-topic and I stop here.

But thanks for the laugh. It was refreshing.

@ckerr
Copy link
Member

ckerr commented May 25, 2024

Looks like those voting in favor of this outnumber those voting against, so let's merge this.

@ckerr ckerr merged commit 381c17e into transmission:main May 25, 2024
24 checks passed
@tearfur tearfur deleted the webui-speed-label branch May 26, 2024 01:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notes:none Should not be listed in release notes scope:web type:ui
Development

Successfully merging this pull request may close these issues.

None yet

8 participants