[Search][DLS] UX improvements #193945

JoseLuisGJ · 2024-09-25T08:51:21Z

Summary

This PR add several changes ir order to bring more consistency and better UX managing Content Indices and Access Control Indices:

The AccessControlIndexSelector will have always a min-width to display without line breaks the content for better reading
Remove ~~Browse documents~~ title in Documents section and ~~Sync rules~~.
Same gutter and space for each tab section content using a more condensed design
~~Index~~ Mappings tabbed name section has been renamed simple Mappings
The Overview table sync now is managed by the same <AccessControlIndexSelector/> as it is in Documents and Mappings sections
The <AccessControlIndexSelector/> componen now can be more cusomized, passing custom title and description, error status, fullWidth and some more.
The search_index.tsx file for rendering indices was fixed to use same spacing and tabs size

Checklist

Delete any items that are not applicable to this PR.

Any text added follows EUI's writing guidelines, uses sentence case text and includes i18n support
Documentation was added for features that require explanation or tutorials
Unit or functional tests were updated or added to match the most common scenarios
Flaky Test Runner was used on any tests changed
Any UI touched in this PR is usable by keyboard only (learn more about keyboard accessibility)
Any UI touched in this PR does not create any new axe failures (run axe in browser: FF, Chrome)
If a plugin configuration key changed, check if it needs to be allowlisted in the cloud and added to the docker list
This renders correctly on smaller devices using a responsive layout. (You can test this in your browser)
This was checked for cross-browser compatibility

Risk Matrix

Delete this section if it is not applicable to this PR.

Before closing this PR, invite QA, stakeholders, and other developers to identify risks that should be tested prior to the change/feature release.

When forming the risk matrix, consider some of the following examples and how they may potentially impact the change:

Risk	Probability	Severity	Mitigation/Notes
Multiple Spaces—unexpected behavior in non-default Kibana Space.	Low	High	Integration tests will verify that all features are still supported in non-default Kibana Space and when user switches between spaces.
Multiple nodes—Elasticsearch polling might have race conditions when multiple Kibana nodes are polling for the same tasks.	High	Low	Tasks are idempotent, so executing them multiple times will not result in logical error, but will degrade performance. To test for this case we add plenty of unit tests around this logic and document manual testing procedure.
Code should gracefully handle cases when feature X or plugin Y are disabled.	Medium	High	Unit tests will verify that any feature flag or plugin combination still results in our service operational.
See more potential risk examples

For maintainers

This was checked for breaking API changes and was labeled appropriately

efegurkan

Looks good in general.

There are a few changes needed before merging.

packages/kbn-search-index-documents/components/documents_overview.tsx

...ents/search_index/components/access_control_index_selector/access_control_index_selector.tsx

...ublic/applications/enterprise_search_content/components/search_index/sync_jobs/sync_jobs.tsx

efegurkan · 2024-09-27T11:40:24Z

...ublic/applications/enterprise_search_content/components/search_index/sync_jobs/sync_jobs.tsx

+                    defaultMessage: 'Browse document level security syncs',
+                  }
+                ),
+                error: errorOnAccessSync ? true : false,


NitPick- non blocking: errorOnAccessSync and errorOContentSync should be enough. Ternary operator ( something ? return1 : return2;) is not necessary

Makes sense. Already fixed.

efegurkan

LGTM, one small nitpick

mdefazio

Nice job on this and cleaning up some UI things.

I think we will want to revisit the mappings page layout so the dropdown doesn't sit above the rest of the other options. But I know that might be a bigger change than it seems.

My other question is around the copy here:

Do we want to always say Access control? Or Document level security?

I also think the description is a little vague. Is this dropdown simply targeting either the content index or the DLS index in all the various tabs? I wonder if we can then be more explicit in the description or title with this.

For example:

Overview:
Content syncs: Browse recent syncs with the content index
Access control syncs: Browse recent syncs with the document level security index

Documents:
Content index: Browse documents from the content index
Access control index: Browse documents from the document level security index

Let me know if you've discussed this already or if you think these are not blockers and I'll approve so we can move forward.

Nice work on getting more PRs in!!

JoseLuisGJ · 2024-09-27T13:59:30Z

Good point @mdefazio. I'm curious what @leemthompo thinks about this.
cc: @danajuratoni @seanstory @jedrazb

seanstory · 2024-09-30T18:29:22Z

Do we want to always say Access control? Or Document level security?

"Access Control" is the type of sync, and "Access Control Lists" are the output of one of those syncs.

These are used for the Document-Level Security feature.

My suggestion would be:

Overview:
Content syncs: Browse content sync history
Access control syncs: Browse access control sync history

Documents:
Content index: Browse documents ingested by your content syncs
Access control index: Browse access control lists ingested by your access control syncs

I'm not sure I'd mention DLS/Document Level Security in any it.

leemthompo · 2024-10-01T10:57:24Z

Big ++ to @seanstory's suggestions.

If we wanted to link access control syncs to DLS maybe a tooltip would be a lightweight way of doing so? :)

JoseLuisGJ · 2024-10-01T11:36:58Z

This is how it looks like this tooltip suggested by @leemthompo

mdefazio

Thanks for making the updates!

daveyholler

Thanks for jumping on this, Jose!

danajuratoni · 2024-10-02T08:48:35Z

Big ++ to @seanstory's suggestions.

If we wanted to link access control syncs to DLS maybe a tooltip would be a lightweight way of doing so? :)

Personally I find tooltips make an already complex page (with drop-down, table, etc ) even busier, but that's just my 2 🪙
I also like Sean's suggestions, with one simplification. We can remove "your" from the descriptions on the Documents view, and make them aligned with the Overview descriptions.

Documents:
Content index: Browse documents ingested by ~~your~~ content syncs
Access control index: Browse access control lists ingested by ~~your~~ access control syncs

leemthompo · 2024-10-02T12:04:56Z

Agree with @danajuratoni's copy edits and I also think the tooltip might be a little clunky in this spot now that I see the preview.

JoseLuisGJ · 2024-10-02T12:26:04Z

For the tooltip after checking how it feels. I guess we should use the question mark pattern as we do with the underlying table, this feels less intrusive rather than triggering the tooltip on hovering hover the Select component:

efegurkan · 2024-10-04T10:49:14Z

@elasticmachine merge upstream

kibana-ci · 2024-10-04T12:37:43Z

💛 Build succeeded, but was flaky

Buildkite Build
Commit: cb8af11

Failed CI Steps

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
`enterpriseSearch`	2.6MB	2.6MB	+1.1KB
`serverlessSearch`	331.4KB	331.3KB	-183.0B
total			+962.0B

History

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

cc @JoseLuisGJ

kibanamachine · 2024-10-04T13:57:22Z

Starting backport for target branches: 8.x

https://github.com/elastic/kibana/actions/runs/11181274990

## Summary This PR add several changes ir order to bring more consistency and better UX managing _Content Indices_ and _Access Control Indices_: - The `AccessControlIndexSelector ` will have always a `min-width` to display without line breaks the content for better reading - Remove ~~_Browse documents_~~ title in Documents section and ~~_Sync rules_~~. - Same gutter and space for each tab section content using a more condensed design - _~~Index~~ Mappings_ tabbed name section has been renamed simple _Mappings_ - The Overview table sync now is managed by the same `<AccessControlIndexSelector/>` as it is in _Documents_ and _Mappings_ sections - The `<AccessControlIndexSelector/>` componen now can be more cusomized, passing custom title and description, error status, _fullWidth_ and some more. - The `search_index.tsx` file for rendering indices was fixed to use same spacing and tabs size ![CleanShot 2024-09-24 at 15 41 47](https://github.com/user-attachments/assets/02bf764d-672a-4b7b-b861-23d98502b912) --------- Co-authored-by: Elastic Machine <[email protected]> (cherry picked from commit 4e65fd2)

kibanamachine · 2024-10-04T14:02:48Z

💚 All backports created successfully

Status	Branch	Result
✅	8.x

Note: Successful backport PRs will be merged automatically after passing CI.

Questions ?

Please refer to the Backport tool documentation

# Backport This will backport the following commits from `main` to `8.x`: - [[Search][DLS] UX improvements (#193945)](#193945)  ### Questions ? Please refer to the [Backport tool documentation](https://github.com/sqren/backport)  Co-authored-by: José Luis González <[email protected]>

## Summary This PR add several changes ir order to bring more consistency and better UX managing _Content Indices_ and _Access Control Indices_: - The `AccessControlIndexSelector ` will have always a `min-width` to display without line breaks the content for better reading - Remove ~~_Browse documents_~~ title in Documents section and ~~_Sync rules_~~. - Same gutter and space for each tab section content using a more condensed design - _~~Index~~ Mappings_ tabbed name section has been renamed simple _Mappings_ - The Overview table sync now is managed by the same `<AccessControlIndexSelector/>` as it is in _Documents_ and _Mappings_ sections - The `<AccessControlIndexSelector/>` componen now can be more cusomized, passing custom title and description, error status, _fullWidth_ and some more. - The `search_index.tsx` file for rendering indices was fixed to use same spacing and tabs size ![CleanShot 2024-09-24 at 15 41 47](https://github.com/user-attachments/assets/02bf764d-672a-4b7b-b861-23d98502b912) --------- Co-authored-by: Elastic Machine <[email protected]>

JoseLuisGJ added 5 commits September 25, 2024 09:45

AccessControlIndexSelector component can be more customized

8f8be5c

Consuming always AccessControlIndexSelector with new props

280957a

Index mappings becomes simple Mappings and index spaces fix

18ae24e

Removing Sync rules tabs section title

44bd704

Better text hierarchy treatment

fe47e7a

JoseLuisGJ added the v8.16.0 label Sep 25, 2024

JoseLuisGJ self-assigned this Sep 25, 2024

efegurkan reviewed Sep 26, 2024

View reviewed changes

JoseLuisGJ added 3 commits September 27, 2024 10:59

Using emotion for min-width accessControlSwitch container

e7c84d8

Using euiTheme.base for accessControlSwitch container

0d4e41c

Passing indexSelectorOptions to the AccessControlIndexSelector component

a193a2d

JoseLuisGJ marked this pull request as ready for review September 27, 2024 10:05

JoseLuisGJ requested review from a team as code owners September 27, 2024 10:05

efegurkan reviewed Sep 27, 2024

View reviewed changes

efegurkan approved these changes Sep 27, 2024

View reviewed changes

efegurkan added v9.0.0 Team:EnterpriseSearch release_note:skip Skip the PR/issue when compiling release notes labels Sep 27, 2024

errorOnAccessSync and errorOContentSync more simple prop pass

04a4bd5

efegurkan added the backport:version Backport to applied version labels label Sep 27, 2024

mdefazio requested changes Sep 27, 2024

View reviewed changes

JoseLuisGJ added 2 commits October 1, 2024 13:01

AccessControlIndexSelector text content reviwed

846c80a

Tooltip connecting access control syncs to DLS

cd55edc

mdefazio approved these changes Oct 1, 2024

View reviewed changes

daveyholler approved these changes Oct 1, 2024

View reviewed changes

JoseLuisGJ added 3 commits October 1, 2024 16:48

i18n issue fix

121f874

i18n issue fix

12200c9

i18n fix using different Message ids

1e33334

Fixing i18n issue with unussed trtanslations

1127c48

Content fix and using EuiIconTip tooltip rather than regular Tooltip

c69bdb1

JoseLuisGJ added 2 commits October 2, 2024 17:20

Fixing unit test for documents overview when is empty

860e332

Fixing unit test DocumentsOverview

eb5bf9f

Merge branch 'main' into DLS-UX-improvements

cb8af11

JoseLuisGJ merged commit 4e65fd2 into elastic:main Oct 4, 2024
24 checks passed

kibanamachine mentioned this pull request Oct 4, 2024

[8.x] [Search][DLS] UX improvements (#193945) #195062

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Search][DLS] UX improvements #193945

[Search][DLS] UX improvements #193945

JoseLuisGJ commented Sep 25, 2024 •

edited by kibanamachine

Loading

efegurkan left a comment

efegurkan Sep 27, 2024

JoseLuisGJ Sep 27, 2024

efegurkan left a comment

mdefazio left a comment

JoseLuisGJ commented Sep 27, 2024

seanstory commented Sep 30, 2024

leemthompo commented Oct 1, 2024 •

edited

Loading

JoseLuisGJ commented Oct 1, 2024

mdefazio left a comment

daveyholler left a comment

danajuratoni commented Oct 2, 2024

leemthompo commented Oct 2, 2024

JoseLuisGJ commented Oct 2, 2024

efegurkan commented Oct 4, 2024

kibana-ci commented Oct 4, 2024

kibanamachine commented Oct 4, 2024

kibanamachine commented Oct 4, 2024

[Search][DLS] UX improvements #193945

[Search][DLS] UX improvements #193945

Conversation

JoseLuisGJ commented Sep 25, 2024 • edited by kibanamachine Loading

Summary

Checklist

Risk Matrix

For maintainers

efegurkan left a comment

Choose a reason for hiding this comment

efegurkan Sep 27, 2024

Choose a reason for hiding this comment

JoseLuisGJ Sep 27, 2024

Choose a reason for hiding this comment

efegurkan left a comment

Choose a reason for hiding this comment

mdefazio left a comment

Choose a reason for hiding this comment

JoseLuisGJ commented Sep 27, 2024

seanstory commented Sep 30, 2024

leemthompo commented Oct 1, 2024 • edited Loading

JoseLuisGJ commented Oct 1, 2024

mdefazio left a comment

Choose a reason for hiding this comment

daveyholler left a comment

Choose a reason for hiding this comment

danajuratoni commented Oct 2, 2024

leemthompo commented Oct 2, 2024

JoseLuisGJ commented Oct 2, 2024

efegurkan commented Oct 4, 2024

kibana-ci commented Oct 4, 2024

💛 Build succeeded, but was flaky

Failed CI Steps

Metrics [docs]

Async chunks

History

kibanamachine commented Oct 4, 2024

kibanamachine commented Oct 4, 2024

💚 All backports created successfully

Questions ?

JoseLuisGJ commented Sep 25, 2024 •

edited by kibanamachine

Loading

leemthompo commented Oct 1, 2024 •

edited

Loading