ManageIQ / manageiq-ui-classic

Classic UI of ManageIQ
Apache License 2.0
50 stars 357 forks source link

Whitespace in name/description/etc. not displayed faithfully in UI #7012

Open dmetzger57 opened 4 years ago

dmetzger57 commented 4 years ago

Original BZ: https://bugzilla.redhat.com/show_bug.cgi?id=1797715

Description of problem:

If there are leading, trailing, or multiple consecutive whitespace characters in a record (e.g., Zone name), the UI will not display them.

Version-Release number of selected component (if applicable):

Verified in 5.10 and 5.11.

How reproducible:

100%

Steps to Reproduce: 1.) Create zones with names/descriptions of 'test 1' and 'test 1'. 2.) Verify that the zones both appear as 'test 1' in the settings accordion, as well as on the zone settings form that displays when you click on them.

Actual results:

Unable to distinguish between two zones whose names/descriptions differ in the amount of whitespace.

Expected results:

Fields display without stripping whitespace characters.

Additional info:

Verified that adding the following CSS to the name, description, or treeview elements resolves the issue:

{ white-space: pre; }

This issue shows up elswhere in the web UI; tested and verified on chargeback rates and service catalogs, for example.

h-kataria commented 4 years ago

@skateman @himdel any ideas on how we can handle this, this is caused by spaces getting compacted when html renders text. Should this be considered a bug?

chessbyte commented 4 years ago

@Fryguy should ManageIQ even allow Zone names with leading or trailing whitespace?

h-kataria commented 4 years ago

@chessbyte this issue is more of having multiple spaces between the name or description field like

test 1
vs
test  1
in both cases value on screen gets displayed as test 1

chessbyte commented 4 years ago

@h-kataria why is that? is our UI squeezing out spaces between words? If the user decides to name 1 thing "test 1" and another "test 1", that is their choice. I don't think we should invest too much effort into highlighting these differences in the UI. They can always rename them, I assume.

btw - the first "test 1" has 1 space in it, while the second "test 1" has 2 spaces in it

h-kataria commented 4 years ago

@chessbyte there is nothing special we do to process this data, Data is stored correctly in the database, but issue is happening when these string are displayed in browser, HTML rendering in browser only displays a single space when there are multiple spaces in the string. If you notice in your comment above https://github.com/ManageIQ/manageiq-ui-classic/issues/7012#issuecomment-622410942 both "test 1" strings only display a single space.

chessbyte commented 4 years ago

@h-kataria yes, I do see that - and kind of expect it too. just questioning whether this needs to be fixed in the UI at all.

skateman commented 4 years ago

My idea is to have a CSS class around stuff where we want to display text faithfully and set the right CSS rule* to not collapse spaces. We just need a list of the places where this is necessary. My guesses are:

* I don't remember it at the moment, but there should be one as far as I remember

EDIT: yeah, this is standard HTML behavior, not a bug but a feature of the web

h-kataria commented 4 years ago

My idea is to have a CSS class around stuff where we want to display text faithfully and set the right CSS rule* to not collapse spaces. We just need a list of the places where this is necessary. My guesses are:

* summary screens

* page titles (the h3 on the top of each page after the header)

* GTLs
  • I don't remember it at the moment, but there should be one as far as I remember

EDIT: yeah, this is standard HTML behavior, not a bug but a feature of the web

Would that work in form text fields as well?

skateman commented 4 years ago

Well, form fields should also handle this rule for sure. My guess is white-space, but maybe @himdel knows something better.

himdel commented 4 years ago

Well, white-space: break-spaces should work, at the cost of breaking on newlines in names. (Or rather, doing a line break in places we don't expect, such as headings and breadcrumbs.)

Except we can't quite control the whitespace haml outputs, and it adds newlines. So this would have to be a rails helper wherever we use a name or description. (React is fine, all it needs is the css, but we still need to find all the places.)

(We could also just s/\s/ /g on the strings, which would fix the surprising newlines issue, but that just means no linebreaks for long names, so, not better.)

Also, given the "name/description/etc".. should we just apply this to all the stringlike fields? I mean, it sounds right, but that also means changing breadcrumbs, textual summaries, gtls, and all the custom view screens. EDIT: actually, this probably involves simply touching all the places where a value from database is rendered in html, or rewriting bits of haml & react, right? :) (well, and angular)

himdel commented 4 years ago

Also, supporting random whitespace in fields might mean being able to edit it, which means supporting hybrid input/textareas when editing.

So, yeah, I don't know. How often do people use significant whitespace?

chessbyte commented 4 years ago

I still don't understand the actual need behind this request. Basically, it can be rephrased as an enhancement to "show significant whitespace". I would decline this enhancement request unless someone can give examples of real-world use cases. /cc @Fryguy

Fryguy commented 4 years ago

Only thing I can think of is if someone, for example, copy-pastes from the UI and searches in the logs or db via a sql query, then they wont find it, which is super confusing. Having things different in the UI vs logs or in the db can create support headaches.

We actually had a very similar real world issue occur recently and it took like 5 engineers to figure out that there was magic whitespace in there (in this particular issue's case it was provisioning a VM with a leading space, but on our side it was stripped and it was very difficult to figure out why the database "didn't have it" even though it did. cc @agrare)