Skip to content
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.

Sign up for GitHub

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

Jump to bottom

[core] Upgrade monorepo #6549

Merged
merged 6 commits into from
Oct 20, 2022
Merged

[core] Upgrade monorepo #6549

merged 6 commits into from
Oct 20, 2022

Conversation

cherniavskii
Copy link
Member

@cherniavskii cherniavskii commented Oct 18, 2022 •
edited
Loading

@cherniavskii cherniavskii changed the title [core] Upgrade monorepo [core] Update monorepo Oct 18, 2022
@cherniavskii cherniavskii changed the title [core] Update monorepo [core] Upgrade monorepo Oct 18, 2022
@cherniavskii cherniavskii added the core Infrastructure work going on behind the scenes label Oct 18, 2022
cherniavskii
cherniavskii commented Oct 18, 2022
View reviewed changes
docs/next.config.js
@@ -43,6 +43,14 @@ module.exports = withDocsInfra({
return {
...config,
plugins,
resolve: {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See mui/material-ui#32107 (comment)
I don't really understand why this change is necessary

@mui-bot
Copy link

mui-bot commented Oct 18, 2022 •
edited
Loading

Messages
📖 Netlify deploy preview: https://deploy-preview-6549--material-ui-x.netlify.app/

These are the results for the performance tests:

Test case Unit Min Max Median Mean σ
Filter 100k rows ms 498.1 858.5 702.3 676.96 154.498
Sort 100k rows ms 563.4 1,003.7 761.2 813.18 146.782
Select 100k rows ms 200.9 296.3 213 226.48 35.372
Deselect 100k rows ms 143.3 282.4 193.2 192.28 49.937

Generated by 🚫 dangerJS against 58f48ce

cherniavskii
cherniavskii commented Oct 18, 2022
View reviewed changes
.eslintrc.js
@@ -77,6 +77,7 @@ module.exports = {
'jsdoc/require-returns': ['error', { contexts: ['TSFunctionType'] }],
'jsdoc/require-returns-type': ['error', { contexts: ['TSFunctionType'] }],
'jsdoc/require-returns-description': ['error', { contexts: ['TSFunctionType'] }],
'react/no-unstable-nested-components': ['error', { allowAsProps: true }],
Copy link
Member Author

@cherniavskii cherniavskii Oct 18, 2022 •
edited by oliviertassinari
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To fix new eslint errors https://app.circleci.com/pipelines/github/mui/mui-x/26220/workflows/f133aad6-a5b3-405b-815d-86526f207584/jobs/150743.

See mui/material-ui#34518 and https://github.com/jsx-eslint/eslint-plugin-react/blob/master/docs/rules/no-unstable-nested-components.md#rule-options for more details.

@cherniavskii cherniavskii mentioned this pull request Oct 19, 2022
Merged
2 tasks
m4theushw
m4theushw reviewed Oct 19, 2022
View reviewed changes
Copy link
Member

@m4theushw m4theushw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The demo in https://deploy-preview-6549--material-ui-x.netlify.app/x/react-data-grid/layout/#predefined-dimensions has a fixed-height div wrapping the div that is shown in the preview. If the height of the inner div is changed in the preview, the layout breaks.

image

@cherniavskii
Copy link
Member Author

@m4theushw I think we can just remove the wrapper div. I'm not sure what was the original intention there

@alexfauquette
Copy link
Member

It does not surprise me that setting a height too large implies breaking the doc layout. It's interesting to have access to this value quickly, to get the following interaction:

  • Ok, it's fixed height demo
  • What append if I modify the height?
  • Ok, it removes the scroll bar and adds empty space between the last row and the footer

@cherniavskii
Copy link
Member Author

Apparently wrapper div was there to make inner div to be included in demo preview:
Screenshot 2022-10-19 at 18 47 36

So I kept the wrapper div, but removed hardcoded height.

It does not surprise me that setting a height too large implies breaking the doc layout

Yeah, but if we can avoid it - why not? :)

@cherniavskii cherniavskii merged commit 43c3ca0 into mui:next Oct 20, 2022
@cherniavskii cherniavskii deleted the upgrade-monorepo branch October 20, 2022 09:35
cherniavskii added a commit to cherniavskii/mui-x that referenced this pull request Oct 20, 2022
@cherniavskii
[core] Upgrade monorepo (mui#6549)
0d1834b
@cherniavskii cherniavskii mentioned this pull request Oct 20, 2022
Merged
@oliviertassinari oliviertassinari mentioned this pull request Oct 21, 2022
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@m4theushw m4theushw m4theushw approved these changes

@alexfauquette alexfauquette Awaiting requested review from alexfauquette

@MBilalShafi MBilalShafi Awaiting requested review from MBilalShafi

Assignees
No one assigned
Labels
core Infrastructure work going on behind the scenes
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@cherniavskii @mui-bot @alexfauquette @m4theushw