-
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
Upgrade EUI to v92.1.1 #174955
Upgrade EUI to v92.1.1 #174955
Changes from all commits
c049395
a9b3350
8af1233
abe9c31
fefb099
6b5aace
443396e
f4f209f
c63770e
7fd36a9
8435765
71ef042
54262d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -102,7 +102,7 @@ | |
"@elastic/datemath": "5.0.3", | ||
"@elastic/elasticsearch": "npm:@elastic/[email protected]", | ||
"@elastic/ems-client": "8.5.1", | ||
"@elastic/eui": "92.0.0-backport.0", | ||
"@elastic/eui": "92.1.1", | ||
"@elastic/filesaver": "1.1.2", | ||
"@elastic/node-crypto": "1.2.1", | ||
"@elastic/numeral": "^2.5.1", | ||
|
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -85,7 +85,7 @@ export const LICENSE_OVERRIDES = { | |
'[email protected]': ['Eclipse Distribution License - v 1.0'], // cf. https://github.com/bjornharrtell/jsts | ||
'@mapbox/[email protected]': ['MIT'], // license in readme https://github.com/tmcw/jsonlint | ||
'@elastic/[email protected]': ['Elastic License 2.0'], | ||
'@elastic/eui@92.0.0-backport.0': ['SSPL-1.0 OR Elastic License 2.0'], | ||
'@elastic/eui@92.1.1': ['SSPL-1.0 OR Elastic License 2.0'], | ||
'[email protected]': ['CC-BY-4.0'], // retired ODC‑By license https://github.com/mattcg/language-subtag-registry | ||
'[email protected]': ['MIT'], // license in importing module https://www.npmjs.com/package/binary | ||
'@bufbuild/[email protected]': ['Apache-2.0'], // license (Apache-2.0 AND BSD-3-Clause) | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -32,6 +32,10 @@ | |
* Resizer | ||
*/ | ||
|
||
.visEditor__resizer { | ||
height: auto; | ||
} | ||
|
||
.visEditor__resizer-isHidden { | ||
display: none; | ||
} | ||
|
Original file line number | Diff line number | Diff line change | ||||
---|---|---|---|---|---|---|
|
@@ -109,6 +109,7 @@ function DefaultEditor({ | |||||
</EuiResizablePanel> | ||||||
|
||||||
<EuiResizableButton | ||||||
alignIndicator="start" | ||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @elastic/kibana-visualizations I noticed while doing a quick grep through Kibana for
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanx Cee! I really appreciate it |
||||||
className={`visEditor__resizer ${isCollapsed ? 'visEditor__resizer-isHidden' : ''}`} | ||||||
/> | ||||||
|
||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -52,15 +52,17 @@ export const getSavedQueriesComplexTest = () => | |
|
||
// hidden columns | ||
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns35'); | ||
cy.getBySel('dataGridHeaderCell-osquery.cmdline').click(); | ||
cy.contains(/Hide column$/).click(); | ||
cy.getBySel('dataGridHeaderCell-osquery.cwd').click(); | ||
|
||
cy.contains(/Hide column$/).click(); | ||
cy.getBySel('dataGridHeaderCell-osquery.disk_bytes_written.number').click(); | ||
|
||
cy.contains(/Hide column$/).click(); | ||
cy.getBySel('dataGridColumnSelectorButton').click(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. So the current behavior doesn't require clicking the header first then 'Hide column' button ? But instead using a specific selector? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, see above - for some reason trying to click the header cell actions was timing out Cypress (I could reproduce this locally, although I honestly have no idea why it was happening) and causing Chrome Renderer crashes. So I opted to use the selector toolbar button instead. EUI already has its own component Cypress tests for hiding columns via the header actions buttons, so hopefully we would catch any regressions around that UX ourselves before they ever reach Kibana. |
||
cy.get('[data-popover-open="true"]').should('be.visible'); | ||
cy.getBySel('dataGridColumnSelectorToggleColumnVisibility-osquery.cmdline').click(); | ||
cy.getBySel('dataGridColumnSelectorToggleColumnVisibility-osquery.cwd').click(); | ||
cy.getBySel( | ||
'dataGridColumnSelectorToggleColumnVisibility-osquery.disk_bytes_written.number' | ||
).click(); | ||
cy.getBySel('dataGridColumnSelectorButton').click(); | ||
cy.get('[data-popover-open="true"]').should('not.exist'); | ||
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns32/35'); | ||
|
||
// change pagination | ||
cy.getBySel('pagination-button-next').click().wait(500).click(); | ||
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns32/35'); | ||
|
@@ -72,8 +74,7 @@ export const getSavedQueriesComplexTest = () => | |
cy.getBySel(RESULTS_TABLE_BUTTON).click(); | ||
|
||
// sorting | ||
cy.getBySel('dataGridHeaderCell-osquery.egid').click(); | ||
|
||
cy.getBySel('dataGridHeaderCellActionButton-osquery.egid').click({ force: true }); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why do we need There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Cypress was being really flaky about these clicks (I'm not totally sure why as they work just fine in production, I think it's either layout settling or rerenders due to how fast it "clicks" things). CI was consistently failing for me without the |
||
cy.contains(/Sort A-Z$/).click(); | ||
cy.getBySel(RESULTS_TABLE_COLUMNS_BUTTON).should('have.text', 'Columns32/35'); | ||
cy.getBySel(RESULTS_TABLE_BUTTON).trigger('mouseover'); | ||
|
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.
@davismcphee I'm hoping you can approve/review these changes to the package/Discover layout since we've been discussing this last week! 🙏 Please do let me know if you find any issues or aren't a huge fan of the renames, etc.
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 changes look and work great, thanks! The renames/cleanups look good too, and I like that the window listener approach means we no longer have to work around
react-reverse-portal
swallowing our events (but keeping the overlay for tooltips, etc. makes sense).