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

chore: improved filterbar for narrowed viewports in web client #6761

Merged
merged 4 commits into from
May 24, 2024

Conversation

Rukario
Copy link
Contributor

@Rukario Rukario commented Mar 31, 2024

Features and fixes ported from #5828 for 4.0.x release

Resolves #6375

Notes: General UI improvement related to filterbar and fixes download/upload speed info wrap.

Left: This PR, Right: Original
Transmission 4 0 x

@Pentaphon
Copy link

Perhaps this should be made consistent with #6739 by putting the arrows to the right as @tearfur has done?

@Rukario
Copy link
Contributor Author

Rukario commented Mar 31, 2024

@Pentaphon Ideally this PR jointed with a 4.0.x port of #6739 that would change the layout to however we want for the 4.0.x release

@tearfur tearfur self-requested a review March 31, 2024 13:45
@ckerr ckerr added the needs update The PR has needs to be updated by the submitter label Mar 31, 2024
Copy link
Member

@ckerr ckerr left a comment

Choose a reason for hiding this comment

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

Minor branch shear w / web/assets/css/transmission-app.scss

@Rukario
Copy link
Contributor Author

Rukario commented Mar 31, 2024

Oh, it looks like another PR (#6570) was merged that isn't complied with narrowed viewport this PR tried to resolve. What should I do from this point?

@ckerr
Copy link
Member

ckerr commented Mar 31, 2024

@tearfur wdyt?

@tearfur
Copy link
Member

tearfur commented Apr 1, 2024

I think this PR should overwrite the changes made in #6570. Maybe we can just revert it, then the git conflict will be gone.

We reached a consensus in #6739 over the solution for stopping the transfer speed info from moving around as the text changes, and that requires merging this PR, then porting #6739 to 4.0.x.

@ckerr
Copy link
Member

ckerr commented Apr 1, 2024

@Rukario ^

@Rukario
Copy link
Contributor Author

Rukario commented Apr 2, 2024

Ok, overwrote changes made in another PR

Have we really reached consensus for fixed width though?

@tearfur
Copy link
Member

tearfur commented Apr 5, 2024

Well, IMO the bottom line is regardless of how the "fixed width" discussion develops, this PR should be merged.

@Coeur Coeur removed the needs update The PR has needs to be updated by the submitter label Apr 23, 2024
@Coeur Coeur requested a review from ckerr April 23, 2024 15:37
@Coeur Coeur added this to the 4.0.x milestone Apr 23, 2024
@ckerr
Copy link
Member

ckerr commented May 24, 2024

@Rukario this has a little bit of shear with your original PR -- you removed the width property for speed-dn-label and speed-up-label, and there's also #6570 which widened it to 100px and set white-space: nowrap

I've preserved 6570 here but TBH I'm not sure if it's still needed with your other changes, so please feel free to LMK or fix my meddling in a followup PR ❤️

@ckerr ckerr merged commit 09e68a0 into transmission:4.0.x May 24, 2024
@Rukario
Copy link
Contributor Author

Rukario commented May 24, 2024

I thought a overwrite would have done the job? Maybe I don't understand GitHub too well. This PR is a direct backport of another PR with a fix that has been merged into master branch. This PR and #6570 fix the same issue but doesn't need both, that would make this PR a compromised port.

@tearfur
Copy link
Member

tearfur commented May 27, 2024

Yes, this PR is supposed to overwrite #6570 as I understand.

I'll be porting #6739 over to 4.0.x in the coming few days. I can fix the shear there.

@Rukario Rukario deleted the better-filterbar branch May 29, 2024 01:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

None yet

5 participants