[DataGrid] Improve sizing logic #350
Conversation
oliviertassinari
force-pushed
the
fix-autosize-issue
branch
2 times, most recently
from
d97fb30
to
060eeae
Compare
| @@ -161,7 +172,7 @@ export const GridComponent = React.forwardRef<HTMLDivElement, GridComponentProps | |||
| ); | |||
|
|
|||
| return ( | |||
| <AutoSizerWrapper onResize={debouncedOnResize} style={{ height: 'unset', width: 'unset' }}> | |||
There was a problem hiding this comment.
The fix is to remove style={{ height: 'unset', width: 'unset' }}
oliviertassinari
force-pushed
the
fix-autosize-issue
branch
from
060eeae
to
88e0f16
Compare
oliviertassinari
force-pushed
the
fix-autosize-issue
branch
from
88e0f16
to
597c258
Compare
oliviertassinari
added
bug 🐛
oliviertassinari
force-pushed
the
fix-autosize-issue
branch
from
5f6a434
to
2188df0
Compare
| (appender as any)[method](`[${name}] - ${message}`, ...rest); | ||
|
|
||
| // Don't console log non errors in production | ||
| if (appender === console && method !== 'error' && process.env.NODE_ENV === 'production') { |
There was a problem hiding this comment.
that's not necessary, we use the factory to initialize the logger and we want to log errors in prod as well
There was a problem hiding this comment.
I think that it's important for 1. of #350 (comment).
If the message should be logged in production, then it should be an error, not a warning. I forgot to add forceDebug
| if (appender === console && method !== 'error' && process.env.NODE_ENV === 'production') { | |
| if ( | |
| appender === console && | |
| !forceDebug && | |
| method !== 'error' && | |
| process.env.NODE_ENV === 'production' | |
| ) { |
There was a problem hiding this comment.
IMHO the logging level should not change between prod and dev. Otherwise ppl will have an error in prod and not in dev and they will be like WTF
There was a problem hiding this comment.
Otherwise ppl will have an error in prod and not in dev and they will be like WTF
This can't happen in the current configuration. All the errors reported in production as also reported in development. Warn in dev, error only in prod is how React and Material-UI behaves (and have been since the beginning), developers are used to it. I think that the surprise will be if we report warnings in prod.
IMHO the logging level should not change between prod and dev.
Oh, I didn't think about that. It could be cleaner. We could have the log level as a warning in dev and error in production, instead of the custom forking logic. This sounds better and more consistent! I can do that. Thanks for the idea :)
Closes #344. I have refactored AutoSizer from classes to hooks for free. It was interesting to dive into the logic and fix the integration with the grid (of course I have made mistakes during the migration that took me almost an hour to identify).