Added restrictWidth to EuiPage
#896
Conversation
- Breaking: the default is to max-width the page at 1000px
src/components/page/page.js
Outdated
| * set to `false` to not restrict the width, | ||
| * set to a number for a custom width. | ||
| */ | ||
| restrictWidth: PropTypes.any, |
There was a problem hiding this comment.
@chandlerprall I'm not sure if this is particularly correct, but we want to accept either a bool or string, maybe a custom prop type, but that seemed excessive...
There was a problem hiding this comment.
You could also do this to be a little more specific
restrictWidth: PropTypes.oneOfType([
PropTypes.bool,
PropTypes.number
]),
There was a problem hiding this comment.
Ahh perfect, I'll do that.
There was a problem hiding this comment.
I'm good with the spirit of this and agree we need something. I think we might want to default to false simply to avoid the breaking change since it's a heavily used component and saves people downstream some headache (thinking about @gjones).
Right now it's really pretty split in Kibana over which pages go full width and which don't, so I think it would require an equal amount of work to maintain no matter which was the default.
|
Thanks for bringing attention to this @snide, yeah I’d definitely agree with setting this prop to false default. Other than adding more work on our end, I’m not sure I personally agree with the library being so opinionated on such a divisive size. Although I trust and leave those decisions to your team. |
|
That's fine, I'll set the default to "false". I was mainly thinking of Kibana where most of the implementation so far have needed it. |
and fixing strings to be numbers
Sets the max-width of the page, set to
trueto use the default size, set tofalseto not restrict the width (default), set to a number for a custom width.You can see the change in the Doc
app_view.js.