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.

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

Mantine 7 support #5340

Closed
wants to merge 35 commits into from
Closed

Mantine 7 support #5340

wants to merge 35 commits into from

Conversation

glebtv
Copy link

@glebtv glebtv commented Dec 1, 2023

closes #5178

Notable problems:
Mantine 7 uses CSS modules for styling, and @refinedev/mantine uses tsup to build, which doesn't support css modules
egoist/tsup#536

Any input or feedback welcome

Mantine migration guides:
https://v6.mantine.dev/changelog/6-0-0/
https://mantine.dev/guides/6x-to-7x/

Copy link

changeset-bot bot commented Dec 1, 2023

🦋 Changeset detected

Latest commit: 515dc57

The changes in this PR will be included in the next version bump.

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@aliemir
Copy link
Member

aliemir commented Dec 5, 2023

Hey @glebtv, thank you for the PR! Great work! I'll request reviews from the core members and let's see what we can work on here. I had a quick look and it looks good actually and I think we can provide couple of solutions for the CSS and then we can just update the mantine examples and ship it 🚢

About the styling issues, I think there are couple of solutions that we can try;

  • Maybe try to use as much as the base styles and try to minimize the manual styling to the style props.
  • We can try using a css-in-js solution but I don't think they will work well with the SSR features.
  • If we had to, we can also export a css file from the package and let the users import it in their apps. So we'll use some prefixed classnames for our components and let users be able to use those class names as selectors.

Thanks again for the PR, we'll keep you updated as soon as possible when we have the chance for a detailed review. 🚀

@piccinnigius
Copy link

any update on this? :) thanks for your pr!

@alicanerdurmaz
Copy link
Member

  • also export a css file from the package and let the users import it in their apps. So we'll use some prefixed classnames for our components and let users be able to use those class names as selectors.

Hello @piccinnigius, we are waiting for @glebtv to make the requested changes. if @glebtv is not available and you would like to continue this development, we would appreciate it 🙌

@amirmamaghani
Copy link

Hey @glebtv , any update on this? :) highly appreciated!

@BatuhanW BatuhanW added the help wanted Extra attention is needed label Jan 24, 2024
@BatuhanW
Copy link
Member

Hey @glebtv , any update on this? :) highly appreciated!

Hey @amirmamaghani we are open to contributions from the community, as the original owner of the PR seeems to be unresponsive.

@amirmamaghani
Copy link

Hey @glebtv , any update on this? :) highly appreciated!

Hey @amirmamaghani we are open to contributions from the community, as the original owner of the PR seeems to be unresponsive.

Hey @BatuhanW , is there any how to guide how to work on these subpackages? I quickly tried to run it but I find it difficult to dig through sourcecodes without seeing any Storybook or sth. Thanks!

@BatuhanW
Copy link
Member

Hey @glebtv , any update on this? :) highly appreciated!

Hey @amirmamaghani we are open to contributions from the community, as the original owner of the PR seeems to be unresponsive.

Hey @BatuhanW , is there any how to guide how to work on these subpackages? I quickly tried to run it but I find it difficult to dig through sourcecodes without seeing any Storybook or sth. Thanks!

Hey @amirmamaghani you can check our contribution guide. https://refine.dev/docs/guides-concepts/contributing/

@arloktev
Copy link

arloktev commented Feb 2, 2024

Hi, what exactly do you need help with this PR? Fix checks or something else?

@glebtv
Copy link
Author

glebtv commented Feb 9, 2024

Sorry, I didn't have any time for this PR, but now I have rebased it to current master and am planning to fix the remaining examples soon

@glebtv
Copy link
Author

glebtv commented Feb 12, 2024

I made @test at packages/ui-tests/tsup.config.ts a separate entrypoint so TestWrapper and render function could be replaced to add MantineProvider to the tests.

Unfortunately this breaks tests in other packages because ui-tests start to use test helpers and TestWrapper from the package and not from ui-tests itself

And TestWrapper in ui-tests uses legacy router, as most of ui-tests

A better solution is welcome if anyone has any

@glebtv
Copy link
Author

glebtv commented Feb 14, 2024

Can someone trigger tests and examples build please?

Copy link

nx-cloud bot commented Feb 14, 2024

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 515dc57. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


🟥 Failed Commands
lerna run cypress:run --scope inferencer-mantine -- --record --key --group inferencer-mantine
✅ Successfully ran 28 targets

Sent with 💌 from NxCloud.

@glebtv
Copy link
Author

glebtv commented Feb 15, 2024

Please try the builds again, all tests and examples should work now.

Please note there is now <Select> and <MultiSelect> components in refinedev/mantine, since mantine 7 Select doesn't work with integers as values, and a useMultiSelect since the MultiSelect prop type is incompatible with Select's.

This component is currently a wrapper around mantine's Selelct but can be later replaced with a custom implementation, like https://mantine.dev/combobox/?e=AsyncAutocomplete

But Mantine select is overall not as good as react-select, maybe replace it with react-select?

Also should I rewrite\clean up the commits or will you squash them?

@glebtv
Copy link
Author

glebtv commented Feb 15, 2024

This PR will also fix MUI snapshots, currently they are broken because of random css- classes.

Will update the branch to fix a conflict in snapshots

@glebtv glebtv changed the title WIP: Initial Mantine 7 support Mantine 7 support Feb 15, 2024
@adamfeldman
Copy link

Is this PR on-track to land in the April Refine release?

@BatuhanW BatuhanW changed the base branch from master to releases/april March 28, 2024 09:30
@BatuhanW BatuhanW requested a review from a team as a code owner March 28, 2024 09:30
Copy link

cypress bot commented Mar 29, 2024

Passing run #10884 ↗︎

0 236 32 0 Flakiness 0

Details:

docs(faq): section about network errors in faq (#5789)
Project: refine Commit: 7fddc23c61
Status: Passed Duration: 21:47 💡
Started: Mar 29, 2024 6:15 AM Ended: Mar 29, 2024 6:37 AM

Review all test suite changes for PR #5340 ↗︎

@BatuhanW BatuhanW deleted the branch refinedev:releases/april April 3, 2024 06:46
@BatuhanW BatuhanW closed this Apr 3, 2024
@BatuhanW
Copy link
Member

BatuhanW commented Apr 3, 2024

Hey @glebtv, we've merged releases/april branch, so your PR is closed. Could you re-open your PR?

@BatuhanW BatuhanW modified the milestones: April Release, May Release Apr 3, 2024
@BatuhanW
Copy link
Member

BatuhanW commented Apr 4, 2024

Hey @glebtv sorry for the inconvenience. I've also emailed you.

@mkhennoussi
Copy link

Any news about Mantine 7 support guys ? Thanks !

@glebtv
Copy link
Author

glebtv commented Apr 23, 2024

I'm sorry but currently I don't have time to continue this work, anyone is free to open a new pull using (or not using) my changes

@mkhennoussi
Copy link

@glebtv Thanks for your answer and for your work ! @BatuhanW is Mantine v7 on the roadmap ? as it seems we are very close to support it...

@BatuhanW
Copy link
Member

BatuhanW commented Jun 5, 2024

Hey @glebtv thanks for the contributions! You've come a long way and we appreciate it.

@mkhennoussi We are open to community contributions for this one.

@BatuhanW BatuhanW removed this from the May Release milestone Jun 5, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed to be continued
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[FEAT] Mantine v7 support?