@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.

Powered by GitHub Issue Mirror