-
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
[Infra UI] Table View for Home Page #29192
Conversation
💔 Build Failed |
@makwarth @formgeist This could probably use some design love... Here is the same screen but with the map view They both need the love. |
…des; adding filter to groups; adding pagination; adding sorting
💔 Build Failed |
💔 Build Failed |
💚 Build Succeeded |
@simianhacker @makwarth How about adding a button toggle in the abilities row, so you can toggle between the views? |
Cool @simianhacker ! @formgeist I think the filter element should be just that, and not include ways of changing the visualization of the filtered data. Also, later the filter will likely include the "Hosts", "Kubernetes", "Docker" filter links, so the filter element will be pretty complex on its own over time. I like the icons though. Could we replace the "Switch to X view" copy with those icons? Ideally Icon + text, so it's very explicit. E.g. |
@simianhacker Do you have a screenshot of how the list view looks when grouped by some keyword(s)? |
@makwarth Sure, you mean something like this |
@formgeist Yes, I like that. Thoughts? |
@formgeist This is why I reached out... love the toggle. |
@makwarth Looks good to me 👌 |
LGTM gents 👏 |
@makwarth @formgeist I updated the PR description with the latest screenshots based on the conversation above. Thanks for the input it was super helpful. |
💚 Build Succeeded |
@formgeist How's this |
💚 Build Succeeded |
@simianhacker Looks great 👍 |
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 great 👍
I did find this issue with the display when setting an explicit grouping - the name
and grouping text overlapping.
Otherwise just a typo in the code. It still might be worth someone else checking the code for such a large changeset, everything looks very sane to me, but I might still miss broader context at this point.
The only other thing I wanted to mention was do we want to keep this file naming: components/*waffle*/index.tsx
, now that that component controls switching between the Waffle Map view and the Table view it doesn't make sense to me that it's waffle/index
, as that suggests it always renders a Waffle Map, and the Table isn't a Waffle Map. Maybe something like nodes_overview/index
?
{({ measureRef, content: { width = 0, height = 0 } }) => { | ||
const groupsWithLayout = applyWaffleMapLayout(map, width, height); | ||
return ( | ||
<WaffleMapOuterContiner innerRef={(el: any) => measureRef(el)} data-test-subj="waffleMap"> |
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.
Small typo: WaffleMapOuterContiner
-> WaffleMapOuterContainer
💔 Build Failed |
@elastic/eui-design Looks like there is a bug with the table. These columns are set to truncate the text but the class for the cell has |
💔 Build Failed |
@Kerry350 I addressed your issues, can you give it a quick once over. |
💚 Build Succeeded |
LGTM 👍 |
💔 Build Failed |
💚 Build Succeeded |
* Addding initial table implimentation * Moving waffle map to seperate component; adding contextual menu to nodes; adding filter to groups; adding pagination; adding sorting * Fixing EUI types for EuiInMemoryTable to work for EVERYONE * Adding server plugin for tslint for VIM; Fixing tests * Adding the view switcher * removing dependency * updating yarn.lock * Change padding to use EUI rules * Rename waffle/index to nodes_overview; move table to nodes_overview * Adding missed files in last commit * Adding textOnly to the columns that need special truncation because they are buttons * Fixed an error in the merge * Fixing merge issues
* Addding initial table implimentation * Moving waffle map to seperate component; adding contextual menu to nodes; adding filter to groups; adding pagination; adding sorting * Fixing EUI types for EuiInMemoryTable to work for EVERYONE * Adding server plugin for tslint for VIM; Fixing tests * Adding the view switcher * removing dependency * updating yarn.lock * Change padding to use EUI rules * Rename waffle/index to nodes_overview; move table to nodes_overview * Adding missed files in last commit * Adding textOnly to the columns that need special truncation because they are buttons * Fixed an error in the merge * Fixing merge issues
This PR adds the table view for the home page. It current features:
Here is an example of clicking on the node name to reveal the context menu (same as Waffle Map)
The map view remains unchanged except it now has the switcher