[EuiDataGrid] Toolbar layout configuration API

[cee-chen]

Initial mock: #5080 (comment)

@constancecchen Showcased the state of this configuration in our weekly where we talked about trying to combine this setting with the density setting as to not overload the toolbar with individual options. Here's a mock/prototype for how we could achieve this. I also updated the language and the "toggle" portion for this to be a bit more clear to the user what is going to happen.

https://www.figma.com/proto/RzfYLj2xmH9K7gQtbSKygn/Elastic-UI?page-id=15895%3A196207&node-id=21744%3A266749&viewport=323%2C48%2C0.79&scaling=min-zoom&starting-point-node-id=21744%3A266749

Static:

PR implementation, with caution from Dave that this will break some custom CSS/positioning by the Security and Obs teams: #5334 (comment)

Just a note to make sure you all communicate with the Security and Observability UI teams (@andrew-goldstein and @weltenwort would be a start). I have a feeling the new positioning will break some of their overwrites (specifically I know they abs position some items into that top right corner). This should give them better guidance for how to position those controls (by using the additional props) they need to add. Not a breaking change the way I see it delivered in EUI, but it will break in Kibana based upon some of their usage.

API proposal from @cchaos: #5334 (comment)

The ideal solution is just making the additionalControls more customizable by position. My suggestion is to convert this prop to either accept a ReactNode as it does now that defaults to the position of middleLeft or an object of ReactNodes with position-named keys like left, middleLeft, middleRight, right. (I'm still workshopping these names).

API proposal from @constancecchen: #5334 (comment)

I do think it's worth maybe exploring toolbar API and the way we let users configure it even more. For example, what if we allowed complete control over not just where their custom content goes but also what order they put the sort/column controls in? e.g.,

toolbarConfiguration = {
  leftToolbar: [
    someCustomContent,
    showSortSelector,
    someOtherCustomContent,
    showColumnSelector,
  ],
  rightToolbar: [
    showDisplayConfig,
    someCustomContent,
    showFullscreen,
  ],
}

... and that kills 2 birds with 1 stone in terms of both custom locations and enabling/disabling certain controls? (although, that makes it a little odd if you put the icon-only fullscreen button in the left side next to the buttons with text - this is me just thinking out loud, so likely needs more fleshing out)

[cee-chen]

Responding to @cchaos's comment (#5334 (comment)) here to keep the conversation going:

My quick thoughts on your suggestion are that I really like the simplicity and ease to configure, but I wonder if there's a way to not allow individual placement of our controls.

For instance, they could move the full screen button to be the first item on the left. But this will break consistency for the user as the view different instances of the same component.

Hm, while there's always a balance to be struck between customization vs. consistency, I'm not seeing the consistency issue when we're allowing users to put in any amount of custom content to the left or right of our controls in any case. In theory if someone adds enough custom content to the right of the fullscreen button, then it's no longer in the place users typically expect (top-right-most button in the toolbar) and the UX is no longer consistent in any case. I'd personally lean more towards customization than consistency based on how I'm seeing other teams use the toolbar (e.g. the Discover team literally hides all our buttons and only uses their own).

Also, forgot to tag @chandlerprall in the original issue description so doing so now!

[cchaos]

Yeah I think mainly, I actually want to avoid any content going to the right of the fullscreen option, and lock that location on the far right.

Also what's the scalability of this array method? This feels more like an opt-in than opt-out so when we add more controls, all consumers would need to add this in manually. For instance, if/when we back in that "no. of results" portion, it would never show up by default.

[cee-chen]

Yeah I think mainly, I actually want to avoid any content going to the right of the fullscreen option, and lock that location on the far right.

Maybe I'm confused, but your left/middleLeft/middleRight/right proposal would still allow that wouldn't it? Also, wouldn't that still break Security and Observability's current implementation since they're using position: right? If we absolutely have to be that strict, should we consider moving the full screen button out of the toolbar to its own thing if we don't want consumers moving it? 🤔

Also what's the scalability of this array method? This feels more like an opt-in than opt-out so when we add more controls, all consumers would need to add this in manually. For instance, if/when we back in that "no. of results" portion, it would never show up by default.

If consumers don't pass a custom array (or empty array if they want nothing), we would just fall back to what's in your mock, which would look like this:

toolbarConfiguration = {
  leftToolbar: [
    showResults, // when we add this
    showColumnSelector,
    showSortSelector,
  ],
  rightToolbar: [
    showDisplayConfig,
    showFullscreen,
  ],
}

So DataGrid consumers that don't need a custom toolbar layout will still get new toolbar defaults - but yeah people who pass in a custom layout will need to manually add new controls. IMO, this is how it should work - to use the Discover example where they hide everything but their own custom content, that's basically what they want in any case.

[chandlerprall]

@cchaos do we want to keep the left / right nomenclature, or begin moving toward language/culture-independent words like start / end as the CSS attributes have been doing. Not that we need to implement anything in that area, only considering the verbiage used.

[cchaos]

start/end seems odd in this context.

I wasn't thinking about keeping fullscreen locked to the right in my original proposal. (Basically I'm changing my mind 😆 ).

But if they just want to turn off the sort selector they have to completely re-establish the configuration.

I'm just thinking that it's really tough to change just a single thing using this method. If we wanted to make it complicated, we could accept both which would look something like:

toolbarConfiguration = {
  showResults, // when we add this
  showColumnSelector,
  showSortSelector,
  showFullScreen,
  showDisplayConfig,

 // Or customize:
  leftToolbar: [
    showResults, // when we add this
    showColumnSelector,
    customButton,
    showSortSelector,
  ],
  rightToolbar: [
    customButton, // shows to the left of the display options button
  ],
}

[cchaos]

Sorry I meant to write more on the start/end comment. I think it's odd because startToolbar and endToolbar sound more like actions than position. I'm unsure if in this case you'd completely swap sides when using an RTL language, or it would lower scoped (like button text and icon).

[cee-chen]

I wasn't thinking about keeping fullscreen locked to the right in my original proposal. (Basically I'm changing my mind 😆 ).

Gotcha, sorry for the confusion, and thanks for clarifying!

If we don't want fully customizable toolbar layouts and instead just a limited number of places users can add their custom controls, can I suggest just left and right position options for simplicity (which will prepend their custom nodes to the left of the specified side of the toolbar?)

I think there's a couple data structures we can consider for that approach:

Simple:

additionalControls={{
  left: ReactNode, // If a single ReactNode is passed instead of an object with left/right keys, the react node defaults to 'left'
  right: ReactNode,
}}

Array:

additionalControls={[
  {
     item: ReactNode, // If a single ReactNode is passed instead of an array, it defaults to this
     position: 'left',
  },
  {
     item: ReactNode,
     position: 'right',
  },
  {
     item: ReactNode,
     position: 'right',
  },
]}

Between the two I think I lean towards the simpler left/right obj keys, since ReactNodes can contain multiple nodes (which is how consumers currently add multiple EuiButtons in any case) I don't see a need for an array. Thoughts?

[cchaos]

I agree that I think we can simplify using those keys under additionalControls. The question then becomes where should they render? The right side makes sense to just come before the internal controls on the right.

But for the left, it's a bit of a harder decision. Keeping before the internal controls would keep parity with how it works today and seems the most logical. But when I think about future additions like the proposed "summary" section, we don't want controls coming before that. So where would it go then? Between the summary any columns selector? Or should it just always be after the left side controls?

[cee-chen]

Ahh good call, I keep forgetting about the results!

Or should it just always be after the left side controls?

👍 I'd lean towards this for now and note it as a breaking change from the current behavior (+ clearly document the expected append/prepend positions in our props). I'd be surprised if teams need something significantly more complex than this.

Does this sound good to everyone else in terms of moving ahead?

[cee-chen]

Also, sorry to detour into the left vs right issue again - I just remembered that I actually ended up renaming my CSS for the different sides into a dataControls side (left) and a displayControls side (right) since it seemed to fit the theme of what kinds of controls we were putting where. If we want to avoid left vs right, do we want to consider data vs. display for naming instead?

additionalControls={{
  data: ReactNode,
  display: ReactNode,
}}

TBH, I don't feel super strongly about this and would be fine sticking to left/right, just wanted to throw that option out there 🤔

[cchaos]

I'm good with moving the custom buttons to the right of the left 😆 .

There's more ambiguity in data/display type of naming than just sticking with left/right.

[cee-chen]

Closing and moving this work back into #5334, since the proposed way forward is backwards-compatible with the current API and doesn't require a breaking prop change