Adding middle truncation to warning popovers #794
Conversation
| <span ng-if="content"> | ||
| <span | ||
| dynamic-content="{{content}}" | ||
| dynamic-content="{{content | middleEllipses:350:'...<br>...'}}" |
There was a problem hiding this comment.
Use HTML entity for the ellipsis?
dynamic-content="{{content | middleEllipses:350:'…<br>…'}}"There was a problem hiding this comment.
I agree! Unfortunately, using the html entity caused the second ellipsis to take up its own line in the popover, so I settled for the less semantic three periods. :(
There was a problem hiding this comment.
the fact that this works makes me worried that its possible for us to be injecting error messages with <> in them
There was a problem hiding this comment.
Nevermind, looks like its handling it
There was a problem hiding this comment.
Line 6 below makes this possible, no? Bootstrap's popover documentation says,
Insert HTML into the popover. If false, jQuery's text method will be used to insert content into the DOM. Use text if you're worried about XSS attacks.
There was a problem hiding this comment.
Yes, something else we are doing is sanitizing the error message already.
|
|
D'oh. I had an outdated Bower dependency. That's what I get for rushing to submit the PR before yoga. Is fixed now. |
|
[merge] |
|
Evaluated for origin web console merge up to d0f58d4 |
|
Origin Web Console Merge Results: SUCCESS (https://ci.openshift.redhat.com/jenkins/job/test_pull_requests_origin_web_console/640/) (Base Commit: d7146b5) |
4 participants
Additional fix for https://bugzilla.redhat.com/show_bug.cgi?id=1389658
Popover text exceeding 350 characters will get truncated in the middle. I arbitrarily chose 350. Should it be more/less?
@jwforres or @spadgett, PTAL