[Dashboard] Add maxWidth 's' to Panel Settings flyouts and 'm' to drilldown widths not already specified in dashboard #189009
Conversation
rshen91
changed the title
rshen91
added
Team:Presentation
rshen91
force-pushed
the
fix-dashboard-max-width
branch
from
6b4b58a to
2827d33
Compare
rshen91
changed the title
|
Pinging @elastic/kibana-presentation (Team:Presentation) |
elastic-vault-github-plugin-prod
bot
requested a review
from a team
as a code owner
There was a problem hiding this comment.
It looks like maxWidth doesn't accept 's' | 'm' | 'l' style sizes - only the size prop accepts these. Personally, I think we should do the following:
- The control settings flyout should have a hard coded size of
s- this is more inline with the panel settings flyout, which I think makes sense - The flyout drilldowns should probably still use the
maxSizeprop rather thansize- and for that, we would have to give it a specific width in pixels or percentage. Perhaps500or600?
cc @cqliu1, just in case you have input here on what you think an appropriate size would be :)
| @@ -126,6 +126,7 @@ export class FlyoutCreateDrilldownAction implements Action<EmbeddableApiContext> | |||
| core | |||
| ), | |||
| { | |||
| maxWidth: 500, | |||
There was a problem hiding this comment.
nit Could we make this a constant so that flyout_create_drilldown and flyout_edit_drilldown stay in sync? 👀
Also, I think 500 might be a little small - maybe 600 would be better? Or maybe size of l might work? Leaving this up to you 👍
There was a problem hiding this comment.
Makes sense to me! Changes in 0200bf9
| @@ -75,3 +75,5 @@ export const createDrilldownTemplatesFromSiblings = ( | |||
|
|
|||
| return templates; | |||
| }; | |||
|
|
|||
| export const drilldownMaxWidth = 500; | |||
There was a problem hiding this comment.
Sorry!! Can you rename this const to align with our constant naming convention? And perhaps change its value as shown in #189009 (comment)? (I edited the comment after the original post - sorry if you didn't see that 🙈 )
| export const drilldownMaxWidth = 500; | |
| export const DRILLDOWN_MAX_WIDTH = 500; |
There was a problem hiding this comment.
Ah sorry yes I missed that - fixed in b1c4e50
💚 Build Succeeded
Metrics [docs]Async chunks
Page load bundle
History
To update your PR or re-run it, just comment with: cc @rshen91 |
Summary
Closes #163981
The Dashboard and Panel Settings Flyouts already seem to be fixed but the Controls Settings Flyout did not specify the maxWidth.
The drilldown (manage and create) flyouts did not have a maxWidth specified but based on main, it seems to match medium. To avoid any unclearness, I have added the maxWidth property to the flyout.
The edit and create drilldowns that didn't have specified maxWidths can be toggled to each other so they should be the same. I think there can be a little bit of a debate on which makes the most sense. Although there is some white space to the right of the buttons in the create drilldown I think it makes the most sense for the manage drilldowns to not be cramped and have the maxWidths be medium if possible.
Edit drilldown
Size s makes it look more cramped and breaks the word Discover onto two lines:
Size m
Create drilldown:
I'm leaning towards size 's' for the create drilldown flyout based on the following screenshots:
Size s
Size m
