De-angularize DocViewer table layout#43240
Conversation
…/kibana into aaronoah-discover-table-collapse
- to get rid of angular markup
- will be replaced with jest
💚 Build Succeeded |
flash1293
left a comment
There was a problem hiding this comment.
LGTM, left some smallish comments but nothing really important. Tested in Chrome
| import { flattenHitWrapper } from '../../../../data/public/index_patterns/index_patterns/flatten_hit'; | ||
| import { DocViewTable } from './table'; | ||
|
|
||
| // @ts-ignore |
There was a problem hiding this comment.
nit: Not sure what's preferable, but you can achieve the same thing with } as unknown as IndexPattern
There was a problem hiding this comment.
when I'm using unknown, i get a typescript complaint the next line:
indexPattern.flattenHit
doing it the right way, would mean I'd to add more than 40 props. I've chosen the pragmatic way, but I'm open for improvements :)
src/legacy/core_plugins/kbn_doc_views/public/views/table/table.test.tsx
Outdated
Show resolved
Hide resolved
src/legacy/core_plugins/kbn_doc_views/public/views/table/table_helper.tsx
Outdated
Show resolved
Hide resolved
| width: 160px; | ||
| } | ||
|
|
||
| .kbnDocViewer__actionButton { |
There was a problem hiding this comment.
Really small thing, but the way the buttons are hidden is causing them to "stay" when you have clicked one and navigate away with the mouse which looks kinda strange:
I filtered for present on continent name and then navigated away to the gender row, but the present filter stays focussed and thus visible. Initially I thought this happened on purpose and was some kind of state (if the filter is active, the button is shown all the time() and was confused it disappeared when doing something else.
There was a problem hiding this comment.
agreed, removed the opacity:1 of the elements focus state
FYI: @cchaos & @ryankeairns
src/legacy/core_plugins/kbn_doc_views/public/views/table/table_row.tsx
Outdated
Show resolved
Hide resolved
| /> | ||
| } | ||
| > | ||
| <i |
There was a problem hiding this comment.
Maybe use IconTip here instead? https://elastic.github.io/eui/#/display/tooltip
There was a problem hiding this comment.
good catch, another font awesome usage dependency removed:
src/legacy/core_plugins/kbn_doc_views/public/views/table/table_helper.tsx
Show resolved
Hide resolved
💔 Build Failed |
💔 Build Failed |
| </React.Fragment> | ||
| `); | ||
| }); | ||
| }); |
There was a problem hiding this comment.
FYI @flash1293 Additionally to the changes you suggested, I've added Jest tests for the helper functions
💚 Build Succeeded |
ryankeairns
left a comment
There was a problem hiding this comment.
👍 The new icons looks good! Thanks for removing the fontawesome icons :)
|
@cchaos are there more changes to be done? got still the _1 change requested_state. thx! |
* Convert component to React * EUI-ficate buttons, warnings and collapse button * Add jest tests
Summary
Replaces Angular of the Table Tab that's displayed when you unfold a list entry at Discover, at Discover's Context and Discover's Doc view.
Part of #38646
Checklist
Use
strikethroughsto remove checklist items you don't feel are 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- [ ] This was checked for keyboard-only and screenreader accessibilityFor maintainers
- [ ] This was checked for breaking API changes and was labeled appropriately