Skip to content
This repository has been archived by the owner on Dec 11, 2019. It is now read-only.

Ledger Doesn't Handle Long Titles Neatly #13195

Closed
jonathansampson opened this issue Feb 20, 2018 · 6 comments · Fixed by #13205
Closed

Ledger Doesn't Handle Long Titles Neatly #13195

jonathansampson opened this issue Feb 20, 2018 · 6 comments · Fixed by #13205

Comments

@jonathansampson
Copy link
Collaborator

jonathansampson commented Feb 20, 2018

Description

A Site or YouTube Channel with a sufficiently long title can break the ledger UX.

Steps to Reproduce

  1. Visit https://www.youtube.com/watch?v=SqqqRqA2caA
  2. View other videos on Channel until Creator appears in Ledger

Actual result:
"ThePotatoIsAStarchyTuberousCropFromThePerennialSolanumTuberosumOfTheNightshadeFamilyLOL" breaks the ledger layout.

Expected result:
Creator's name should not break the ledger layout.

Reproduces how often:
Always

Brave Version

0.20.42

about:brave info:

Brave: 0.20.42 
V8: 6.4.388.41 
rev: 096c7cb3c75ebb518f72fc7d24bc2bbaedd50aed 
Muon: 4.8.2 
OS Release: 10.0.16299 
Update Channel: Release 
OS Architecture: x64 
OS Platform: Microsoft Windows 
Node.js: 7.9.0 
Brave Sync: v1.4.2 
libchromiumcontent: 64.0.3282.140

Reproducible on current live release:
Yes.

@jonathansampson jonathansampson added design A design change, especially one which needs input from the design team. feature/rewards labels Feb 20, 2018
@NejcZdovc NejcZdovc self-assigned this Feb 20, 2018
@ryanml
Copy link
Contributor

ryanml commented Feb 20, 2018

Was thinking about taking this on, but then I see @NejcZdovc got it :)

I would motion towards truncating it to n characters and then adding a trailing '...', then adding a title/label property that would show the full value on hover perhaps. Is that what you were thinking?

@NejcZdovc
Copy link
Contributor

@ryanml you can take it, no problem. I was thinking of fixing all of this with css

@NejcZdovc NejcZdovc added this to the Triage Backlog milestone Feb 20, 2018
@ryanml
Copy link
Contributor

ryanml commented Feb 20, 2018

@NejcZdovc got it, thanks. I certainly don't want to over-engineer anything if you have a pure CSS solution though. If you're fine with a truncation solution, I'll open a PR, else, maybe we could go over what stylistic change might work for this. Thanks!

ryanml added a commit to ryanml/browser-laptop that referenced this issue Feb 20, 2018
@ryanml
Copy link
Contributor

ryanml commented Feb 20, 2018

@NejcZdovc this is what I put together here:

Truncating at 45 characters was the most sensible limit before it started bleeding over.

@NejcZdovc NejcZdovc modified the milestones: Triage Backlog, 0.23.x (Nightly Channel) Feb 23, 2018
@NejcZdovc NejcZdovc assigned ryanml and unassigned NejcZdovc Mar 14, 2018
@NejcZdovc NejcZdovc modified the milestones: 0.23.x (Nightly Channel), 0.22.x (Developer Channel) Mar 14, 2018
@NejcZdovc
Copy link
Contributor

pulling this one into 0.22 based on the triage call

@srirambv
Copy link
Collaborator

srirambv commented Mar 22, 2018

Verified on Windows x64

  • 0.22.6 e6ff4ea
  • libchromiumcontent: 65.0.3325.162
  • muon: 5.1.0

image

Verified on macOS 10.12.6 x64 using the following build:

  • 0.22.6 e6ff4ea
  • libchromiumcontent: 65.0.3325.162
  • muon: 5.1.0

screen shot 2018-03-22 at 6 31 42 pm

Verified on Ubuntu 10.10 x64

  • 0.22.7 8bb7e77
  • libchromiumcontent: 65.0.3325.181
  • muon: 5.1.1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants