-
Notifications
You must be signed in to change notification settings - Fork 8.3k
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
[Lens] Trigger a filter action on click in datatable visualization #63840
[Lens] Trigger a filter action on click in datatable visualization #63840
Conversation
392e754
to
8ddf676
Compare
8ddf676
to
632e096
Compare
Thanks for the recorded video @mbondyra !! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thanks!
@AlonaNadler it says 'filter out value' and 'filter for value' - consistent with the tooltips for datatable visualization but of course we can change it! |
The comment about tap behavior was confirming that it works as expected: no change needed for touch devices. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me. I also tested the functionality, layout and how the filters were saved with the vis. 👍 Just a couple things need to be added.
|
||
.lnsDataTable__filter { | ||
opacity: 0; | ||
transition: opacity 250ms ease-in-out; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
transition: opacity 250ms ease-in-out; | |
transition: opacity $euiAnimSpeedNormal ease-in-out; |
transition: opacity 250ms ease-in-out; | ||
} | ||
|
||
.lnsDataTable__cell:hover { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The icons need to reveal on focus as well.
.lnsDataTable__cell:hover { | |
.lnsDataTable__cell:hover, | |
.lnsDataTable__cell:focus-within { |
aria-label={i18n.translate( | ||
'xpack.lens.filterForValueButtonAriaLabel', | ||
{ | ||
defaultMessage: 'Filter for value', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For the aria-labels only, I'm wondering if we should include the value here like Filter for geo.src: AE
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
done!
@elasticmachine merge upstream |
x-pack/plugins/lens/public/datatable_visualization/_visualization.scss
Outdated
Show resolved
Hide resolved
💚 Build SucceededHistory
To update your PR or re-run it, just comment with: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 Thanks for making those changes.
…lastic#63840) * wip: datatable * fix: empty values * fix: empty values * translations * using dataPlugin to get buckets * one more time, passing aggs data * tests: added * feat: new design applied * remove icon * feat: old design * CR corrections * better name * Fix merge issue * fix: design changes * feat: correction * fix: copy changes * Update x-pack/plugins/lens/public/datatable_visualization/_visualization.scss Co-authored-by: Caroline Horn <[email protected]> * Update _visualization.scss Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Wylie Conlon <[email protected]> Co-authored-by: Caroline Horn <[email protected]>
…lastic#63840) * wip: datatable * fix: empty values * fix: empty values * translations * using dataPlugin to get buckets * one more time, passing aggs data * tests: added * feat: new design applied * remove icon * feat: old design * CR corrections * better name * Fix merge issue * fix: design changes * feat: correction * fix: copy changes * Update x-pack/plugins/lens/public/datatable_visualization/_visualization.scss Co-authored-by: Caroline Horn <[email protected]> * Update _visualization.scss Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Wylie Conlon <[email protected]> Co-authored-by: Caroline Horn <[email protected]>
* upstream/master: (315 commits) [APM] Fix failing `ApmIndices` test (elastic#64965) [APM] Fix paths for ts optimization script (elastic#65012) Use HDR for percentiles (elastic#64758) [EPM] fix updates available filter (elastic#64957) [Uptime] Certificates page (elastic#64059) load lens app lazily (elastic#64769) [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954) Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742) [data.search.aggs] Remove legacy aggs APIs. (elastic#64719) Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927) [Discover] Show doc viewer action buttons on focus (elastic#64912) [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932) [Metrics UI] Add inventory metric threshold alerts (elastic#64292) [Canvas] Adds edit menu (elastic#64738) [Canvas] Adds refresh and autoplay options to view menu (elastic#64375) [Lens] Trigger a filter action on click in datatable visualization (elastic#63840) [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450) [APM] Client new platform migration (elastic#64046) [Monitoring] NP Migration complete client cutover (elastic#62908) Ingest Node Pipelines UI (elastic#62321) ...
…or-part-mvp-2 * 'master' of github.com:elastic/kibana: (51 commits) [APM] Fix failing `ApmIndices` test (elastic#64965) [APM] Fix paths for ts optimization script (elastic#65012) Use HDR for percentiles (elastic#64758) [EPM] fix updates available filter (elastic#64957) [Uptime] Certificates page (elastic#64059) load lens app lazily (elastic#64769) [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954) Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742) [data.search.aggs] Remove legacy aggs APIs. (elastic#64719) Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927) [Discover] Show doc viewer action buttons on focus (elastic#64912) [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932) [Metrics UI] Add inventory metric threshold alerts (elastic#64292) [Canvas] Adds edit menu (elastic#64738) [Canvas] Adds refresh and autoplay options to view menu (elastic#64375) [Lens] Trigger a filter action on click in datatable visualization (elastic#63840) [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450) [APM] Client new platform migration (elastic#64046) [Monitoring] NP Migration complete client cutover (elastic#62908) Ingest Node Pipelines UI (elastic#62321) ... # Conflicts: # x-pack/plugins/ingest_pipelines/common/types.ts # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form.tsx # x-pack/plugins/ingest_pipelines/public/application/components/pipeline_form/pipeline_form_fields.tsx # x-pack/plugins/ingest_pipelines/public/shared_imports.ts
* master: (44 commits) onEvent prop for expression component (elastic#64995) [APM] Fix failing `ApmIndices` test (elastic#64965) [APM] Fix paths for ts optimization script (elastic#65012) Use HDR for percentiles (elastic#64758) [EPM] fix updates available filter (elastic#64957) [Uptime] Certificates page (elastic#64059) load lens app lazily (elastic#64769) [legacy/server/config] remove unnecessary deps for simple helper (elastic#64954) Fixed alert Edit flyout shows the error message when one of this actions has a preconfigured action type (elastic#64742) [data.search.aggs] Remove legacy aggs APIs. (elastic#64719) Fixed `AddAlert` flyout does not immediately update state to reflect new props (elastic#64927) [Discover] Show doc viewer action buttons on focus (elastic#64912) [EPM] restrict package install endpoint from installing/updating to old packages (elastic#64932) [Metrics UI] Add inventory metric threshold alerts (elastic#64292) [Canvas] Adds edit menu (elastic#64738) [Canvas] Adds refresh and autoplay options to view menu (elastic#64375) [Lens] Trigger a filter action on click in datatable visualization (elastic#63840) [SIEM][CASE] Refactor Connectors - Jira Connector (elastic#63450) [APM] Client new platform migration (elastic#64046) [Monitoring] NP Migration complete client cutover (elastic#62908) ...
…63840) (#64993) * wip: datatable * fix: empty values * fix: empty values * translations * using dataPlugin to get buckets * one more time, passing aggs data * tests: added * feat: new design applied * remove icon * feat: old design * CR corrections * better name * Fix merge issue * fix: design changes * feat: correction * fix: copy changes * Update x-pack/plugins/lens/public/datatable_visualization/_visualization.scss Co-authored-by: Caroline Horn <[email protected]> * Update _visualization.scss Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Wylie Conlon <[email protected]> Co-authored-by: Caroline Horn <[email protected]> Co-authored-by: Elastic Machine <[email protected]> Co-authored-by: Wylie Conlon <[email protected]> Co-authored-by: Caroline Horn <[email protected]>
Summary
The design is aligned with inspector design, but here hover happens on cell, not on row:
Here's the little demo:
https://www.loom.com/share/6e5dd6fcd196494cb63216eceefaff8e
Closes #61417
Checklist
Delete any items that are not applicable to this PR.