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

[Maps] disable style forms when they are not applied due to other style settings #55858

Merged
merged 7 commits into from
Jan 31, 2020

Conversation

nreese
Copy link
Contributor

@nreese nreese commented Jan 24, 2020

Fixes #52557

This PR disables style forms for styles that are not applied. For example, when border size is zero then border color is not applied. Border color will be disabled and provide a tooltip as to why its disabled. This will make it clearer when styles are getting applied.

Screen Shot 2020-01-24 at 11 15 49 AM

Screen Shot 2020-01-24 at 11 22 58 AM

@nreese nreese added release_note:enhancement [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation v8.0.0 v7.7.0 labels Jan 24, 2020
@nreese nreese requested a review from jsanz January 24, 2020 16:27
@nreese nreese requested review from a team as code owners January 24, 2020 16:27
@elasticmachine
Copy link
Contributor

Pinging @elastic/kibana-gis (Team:Geo)

@miukimiu miukimiu self-requested a review January 24, 2020 16:42
Copy link
Contributor

@miukimiu miukimiu left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! 🎉

@nreese
Copy link
Contributor Author

nreese commented Jan 28, 2020

@elasticmachine merge upstream

@jsanz
Copy link
Member

jsanz commented Jan 28, 2020

Would it be possible to also disable label border color when the size is set to None?

image

@nreese
Copy link
Contributor Author

nreese commented Jan 29, 2020

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Jan 29, 2020

Would it be possible to also disable label border color when the size is set to None?

great idea. I have updated the functionality to disable label border color when label border is set to none.

@kindsun
Copy link
Contributor

kindsun commented Jan 29, 2020

It's possible right now to set the symbol size to 0 which makes a few of the other settings pointless. We could disable these other settings too as you've done with border width, but rather than disable those settings either via this PR or a even new one, should we just set a min value of 1px for symbol size? Is there any plausible reason to have size 0?

@nreese
Copy link
Contributor Author

nreese commented Jan 29, 2020

should we just set a min value of 1px for symbol size? Is there any plausible reason to have size 0?

I think users may want to show labels only so maybe then setting symbol size to 0 makes sense.

@kindsun
Copy link
Contributor

kindsun commented Jan 29, 2020

I think users may want to show labels only so may then setting symbol size to 0 makes sense.

Good point. Then should we disable Fill Color and Symbol Orientation when Symbol Size is set to 0?

@nreese
Copy link
Contributor Author

nreese commented Jan 29, 2020

Then should we disable Fill Color and Symbol Orientation when Symbol Size is set to 0?

That makes sense. I have pushed changes that will disable symbol style properties when symbol size is zero

@nreese nreese requested a review from kindsun January 29, 2020 18:19
@kindsun
Copy link
Contributor

kindsun commented Jan 30, 2020

This is looking really good! One thing I noticed is that it's possible to have a preexisting border that carries over into when the symbol size is 0. Maybe we should either not disable the border width adjustment or set the border to 0 when size equals 0.

image

@nreese
Copy link
Contributor Author

nreese commented Jan 31, 2020

@elasticmachine merge upstream

@nreese
Copy link
Contributor Author

nreese commented Jan 31, 2020

One thing I noticed is that it's possible to have a preexisting border that carries over into when the symbol size is 0. Maybe we should either not disable the border width adjustment or set the border to 0 when size equals 0.

I created issue, #56490, to track. Border should not be displayed when symbol size is zero. Lets move that conversation to a different thread and not couple this PR with that issue.

@nreese nreese requested a review from kindsun January 31, 2020 14:00
@kibanamachine
Copy link
Contributor

💚 Build Succeeded

History

To update your PR or re-run it, just comment with:
@elasticmachine merge upstream

Copy link
Contributor

@kindsun kindsun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

  • tested locally in chrome
  • code review

@nreese nreese merged commit 38c7d3a into elastic:master Jan 31, 2020
nreese added a commit to nreese/kibana that referenced this pull request Jan 31, 2020
…le settings (elastic#55858)

* [Maps] disable style forms when they are not applied due to other style settings

* disable label border color when label border is none

* disable symbol style inputs when symbol size is zero

Co-authored-by: Elastic Machine <[email protected]>
nreese added a commit that referenced this pull request Jan 31, 2020
…le settings (#55858) (#56508)

* [Maps] disable style forms when they are not applied due to other style settings

* disable label border color when label border is none

* disable symbol style inputs when symbol size is zero

Co-authored-by: Elastic Machine <[email protected]>

Co-authored-by: Elastic Machine <[email protected]>
jfsiii pushed a commit to jfsiii/kibana that referenced this pull request Feb 4, 2020
…le settings (elastic#55858)

* [Maps] disable style forms when they are not applied due to other style settings

* disable label border color when label border is none

* disable symbol style inputs when symbol size is zero

Co-authored-by: Elastic Machine <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Maps] disable border color style input when border width is zero
6 participants