@fdellwing opened this Pull Request on June 5th 2018 Contributor

fixes #13035

@fdellwing commented on June 5th 2018 Contributor

There are a lot of tests failing and some very strange screenshots especially this two:

https://builds-artifacts.matomo.org/matomo-org/matomo/3.x-dev/28135/UIIntegrationTest_dashboard1.png
https://builds-artifacts.matomo.org/matomo-org/matomo/3.x-dev/28135/Morpheus_load.png

I'm pretty sure this has nothing to do with me, but I will not sync my screens until this is worked out.


But please explain this: https://builds-artifacts.matomo.org/matomo-org/matomo/3.x-dev/28135/MeasurableManager_add_measurable_view.png

How could the thing I fixxed be already expected and why is it getting rendered wrong?

Just to be clear here: This fix is against the W3C standard HTML 5.2 from December 2017! As it clearly states:

The placeholder attribute represents a short hint (a word or short phrase) intended to aid the user with data entry when the control has no value. A hint could be a sample value or a brief description of the expected format. The attribute, if specified, must have a value that contains no U+000A LINE FEED (LF) or U+000D CARRIAGE RETURN (CR) characters.

Source: https://www.w3.org/TR/html5/forms.html#the-placeholder-attribute

I tested this change successfully in: IE11, FF60, Chrome 66

@Findus23 commented on June 5th 2018 Member

How could the thing I fixxed be already expected and why is it getting rendered wrong?

I can explain this: It is already working correctly with the spaces if the windows is narrow enough.

@diosmosis commented on June 21st 2018 Member

Thanks for creating this PR @fdellwing! Looks like the reason it fails in phantomjs is because it doesn't work in webkit. See, eg, this screenshot in Safari:

image

Also, I'm not sure if .invalid is a good suffix to use, I think it may confuse some users. siteUrl.example might be better. Or maybe just example1.com?

@fdellwing commented on June 22nd 2018 Contributor

Thats bad news, then there is not really a nice way to do this. I would suggest using JS to write the placeholder into the field and removing it if the input gets focus. Sadly I do not have time for this atm.

You can decline this PR if you are more happy with the current version.

@Findus23 commented on June 22nd 2018 Member

@fdellwing What about the more hackish solution of just adding more spaces and replacing the domain with an .example one?

@fdellwing commented on June 22nd 2018 Contributor

Thats not hackish but simple ugly, but yes, I could do that.

@fdellwing commented on June 22nd 2018 Contributor

I just tripled the current spaces for now, can someone check this with an UHD resolution? For 4k we would need even more I think.

@sgiehl commented on June 25th 2018 Member

As we are building those inputs with angular based on the definitions, I'm wondering if we maybe should do the required magic for safari in angular. Afaik all other modern browsers are able to show the newline in textareas. So maybe we could implement a check in JS if the browser "supports" it or not. And if not, replace the newline with whitespaces.
This way we would have a "normal" definition with new lines. And we maybe can someday simply remove the whitespace hack if apple decides to support w3c definitions fully.

@fdellwing commented on June 25th 2018 Contributor

I have no clue of angular, so sadly you have to do that yourself.

This Pull Request was closed on July 2nd 2018
Powered by GitHub Issue Mirror