Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

MousePosition: allow rendering of empty string #12488

Merged
merged 2 commits into from Jul 9, 2021

Conversation

JakobMiksch
Copy link
Contributor

fixes #12482

ol.control.MousePosition
This PR makes it possible to enter an empty string '' as undefinedHTML value. This will finally add an empty string into the target HTML element. The behavior before was that an empty string was recognized as falsey and therefore the default value - a whitespace - was used in the target HTML element.

@tschaub
Copy link
Member

tschaub commented Jul 8, 2021

The tests make it look like the current behavior is intentional:

it('retains the mouse position when undefinedHTML is falsey and mouse moves outside the viewport', function () {

@MoonE
Copy link
Contributor

MoonE commented Jul 8, 2021

See #8109 for the PR introducing the current behavior.

@tschaub
Copy link
Member

tschaub commented Jul 8, 2021

An alternative would be to render an empty element instead of a non-breaking space by default. This would not break applications that are relying on the current documented/tested behavior with respect to undefinedHTML: ''. But it would potentially break styling if an application is relying on the element always having text content (which I think is why the non-breaking space was added).

@tschaub
Copy link
Member

tschaub commented Jul 8, 2021

Thanks for the reminder, @MoonE . After reading through #8109 and #2914, I think this pull request makes sense (maybe a breaking change, but maybe a fix).

If you want to clear the control on mouseout, use the default options (same as undefinedHTML: ' ').

If you want to retain the position on mouseout, pass undefinedHTML: undefined.

If you want to clear the position by rendering an empty element instead of a non-breaking space, pass undefinedHTML: ''.

Kind of subtle and awkward (but I think that is mostly due to the undefinedHTML name).

@tschaub
Copy link
Member

tschaub commented Jul 8, 2021

@JakobMiksch can you update the tests to cover the new behavior (change existing test for empty string and add test for undefined)?

@marcjansen
Copy link
Member

While we are at it, maybe also change the name from undefinedHTML to sth. less confusing?

@tschaub
Copy link
Member

tschaub commented Jul 9, 2021

While we are at it, maybe also change the name from undefinedHTML to sth. less confusing?

I had placeholder in mind (inspired by <input placeholder="foo">). But we might want to wait for a major release for that. Or we could add placeholder, deprecate undefinedHTML, and then later remove it.

@tschaub
Copy link
Member

tschaub commented Jul 9, 2021

@JakobMiksch can you update the tests to cover the new behavior (change existing test for empty string and add test for undefined)?

I pushed new tests in 9e7f5a9.

@tschaub tschaub merged commit a2f4417 into openlayers:main Jul 9, 2021
@tschaub
Copy link
Member

tschaub commented Jul 9, 2021

Thanks for the contribution, @JakobMiksch

@tschaub
Copy link
Member

tschaub commented Jul 9, 2021

While we are at it, maybe also change the name from undefinedHTML to sth. less confusing?

@marcjansen - see #12491. The change there restores the previous (awkward) behavior of undefinedHTML and adds a new placeholder option. This placeholder option can be set to the empty string to render the empty string when the mouse position is not available.

@JakobMiksch
Copy link
Contributor Author

@tschaub thanks for finalizing this PR. I would have tried soon, but you were faster 👍

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

Successfully merging this pull request may close these issues.

MousePosition Control: Allow setting empty string if coordiante is undefined
4 participants