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] Always show solution layers #86053

Merged

Conversation

thomasneirynck
Copy link
Contributor

@thomasneirynck thomasneirynck commented Dec 15, 2020

Summary

Closes #84931

In 7.11, Maps introduced the disabled-state for add-layer cards. This state is used to show layer-types which are supported, but are unavailable in that particular deployment, e.g. due to licensing issues. The disabled state is used to advertise on-prem EMS layers (Requiring enterprise) and the track layer (requiring gold+).

We could use the same disabled state for the solution-layers. It would surface this functionality a lot better in Maps. Now, solution layers are hidden for users who do not have a proper solution-setup.

Showing all facets: (I moved them down to the bottom iso top)

image

Showing only the solution facet:

image

thoughts: @miukimiu @kmartastic @nreese ?

@thomasneirynck thomasneirynck added discuss WIP Work in progress labels Dec 15, 2020
@thomasneirynck thomasneirynck changed the title Maps/show solution layers [Maps] Always show solution layers Dec 15, 2020
@kmartastic
Copy link
Contributor

I like this approach. I also like that the solution layers are moved to the bottom when disabled. One concern: changing the position of a UI element based on when it's enabled or disabled is not cool. Are you proposing a permanent move from top to bottom for the solution layers cards? @thomasneirynck

Curious what @miukimiu thinks about it.

@thomasneirynck
Copy link
Contributor Author

thomasneirynck commented Dec 16, 2020

Are you proposing a permanent move from top to bottom for the solution layers cards

yes, that'd be kind of the idea, move them to the bottom regardless if enabled/disabled. The "Solution"-facet will list them on the top anyway.

It's because of GeoJson-upload being the first one, otherwise the solution-layers wouldn't be sitting next to eachother but on different rows staggered.

@miukimiu
Copy link
Contributor

@thomasneirynck I also like this approach, and I think it makes sense the solution-layers sitting next to each other.

@thomasneirynck thomasneirynck added v7.12.0 v8.0.0 enhancement New value added to drive a business result release_note:enhancement and removed enhancement New value added to drive a business result labels Jan 12, 2021
@thomasneirynck thomasneirynck marked this pull request as ready for review January 12, 2021 16:06
@thomasneirynck thomasneirynck added [Deprecated-Use Team:Presentation]Team:Geo Former Team Label for Geo Team. Now use Team:Presentation and removed WIP Work in progress labels Jan 12, 2021
@elasticmachine
Copy link
Contributor

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

},
disabledReason: i18n.translate('xpack.maps.security.disabledDesc', {
defaultMessage: 'There is no default security index {index}.',
Copy link
Contributor

Choose a reason for hiding this comment

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

This disabled message is not clear enough to notify users what the problem is and how to resolve it. Maybe it should be something more like "Could not find security index pattern {index}.", Then add a sentence about how to get started with security.

Maybe @gchaps can help?

Copy link
Contributor

Choose a reason for hiding this comment

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

You can add text that suggests users go to the Security Overview (which includes a link to the Security getting started guide.). Is the list of index patterns necessary? My suggestion:

Cannot find security index pattern. To get started with Security, go to Security > Overview.

@miukimiu Is there a way to present this text outside of a tooltip, so we can link directly to the Security > Overview page?

Copy link
Contributor

Choose a reason for hiding this comment

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

@gchaps a pattern that comes to my mind is the one we're using for the visualize modal. The card is disabled, but by clicking the badge, we can redirect users to a different page.

Screenshot 2021-01-13 at 17 48 07

The badge could say something like Disabled. On hover the badge, the tooltip could say: "Cannot find security index pattern. Get started with Security".

What do you think @gchaps, @nreese, and @thomasneirynck?

But then would we follow the same pattern for the other disabled cards?

Copy link
Contributor

Choose a reason for hiding this comment

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

I am not sure if a user would expect/know that the badge is a clickable element.

Also, I think the badges emphasize the disabled cards, which should not be the case. If anything, the disabled cards should be deemphasized.

Honestly, if a user gets to this page and Security or Observability are not set up, I doubt that user would have the gumption/permissions to set up Security and Observability so a simple tooltip may be good enough. In the enterprise/corporate market, the user using the map is not going to be the same user setting up data flows and indexes.

@thomasneirynck
Copy link
Contributor Author

thx @miukimiu @nreese

my suggestion would be to leave this PR as is, without adding a hyperlink, and tackling the change of the card in a separate PR. I really like the L&F of adding the badge, but the details might get a little hairy. e.g. in this specific case, the enabled/disabled won't be necessarily based on "license level" . e.g. the tracks-layer enabled/disabled depends on the license, but the observability/securitry layers are not (this is based on whether APM and SIEM are setup correctly).

}
},
disabledReason: i18n.translate('xpack.maps.observability.disabledDesc', {
defaultMessage: 'Cannot find APM index pattern {patternId}',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can you make the APM message more like the security message, something like "Cannot find APM index pattern. To get started with Observably, go to Observability > Overview."

Copy link
Contributor

@nreese nreese left a comment

Choose a reason for hiding this comment

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

LGTM
code review

@thomasneirynck
Copy link
Contributor Author

@elasticmachine merge upstream

@kibanamachine
Copy link
Contributor

💚 Build Succeeded

Metrics [docs]

Async chunks

Total size of all lazy-loaded chunks that will be downloaded as the user navigates the app

id before after diff
maps 2.9MB 2.9MB +908.0B

History

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

@thomasneirynck thomasneirynck merged commit 55d1065 into elastic:master Jan 19, 2021
thomasneirynck added a commit to thomasneirynck/kibana that referenced this pull request Jan 19, 2021
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] show disabled solution cards in add layer menu when solution data is not available
7 participants