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

Add mantine UI package #3622

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
Open

Conversation

pkalisiewicz
Copy link

@pkalisiewicz pkalisiewicz commented Apr 24, 2023

Reasons for making this change

I wanted to add another provider, which is Mantine to allow to use their components withing rjsf library.

If this is related to existing tickets, include links to them as well. Use the syntax fixes #[issue number] (ex: fixes #123).

If your PR is non-trivial and you'd like to schedule a synchronous review, please add it to the weekly meeting agenda: https://docs.google.com/document/d/12PjTvv21k6LIky6bNQVnsplMLLnmEuypTLQF8a-8Wss/edit

Checklist

  • I'm updating documentation
  • I'm adding or updating code
    • I've added and/or updated tests. I've run npm run test:update to update snapshots, if needed.
    • I've updated docs if needed
    • I've updated the changelog with a description of the PR
  • I'm adding a new feature
    • I've updated the playground with an example use of the feature

@heath-freenome
Copy link
Member

@pkalisiewicz Can you fix your build, then I will take a look at the PR

@pkalisiewicz
Copy link
Author

pkalisiewicz commented Apr 26, 2023

@pkalisiewicz Can you fix your build, then I will take a look at the PR

@heath-freenome

Sure, I'm just having an issue with symlinking.
How can I link to the package that isn't yet published?
Can I somehow get it published as a draft, should I link in the playground directly?

I have temporarily commented out code that is adding mantine in playground until, I'll find a solution:)

@heath-freenome
Copy link
Member

@pkalisiewicz Ah, yeah that is an issue with adding new themes. I'm wrapping up a new feature and will take a look at this PR by next Monday. Sorry for the delay

@heath-freenome
Copy link
Member

@pkalisiewicz it looks like there is a typescript issue with your build?

(typescript) Error: /home/runner/work/react-jsonschema-form/react-jsonschema-form/packages/mantine-ui/src/MantineProvider/MantineProvider.tsx(1,53): semantic error TS2307: Cannot find module '@mantine/core' or its corresponding type declarations.

@heath-freenome
Copy link
Member

@pkalisiewicz If you can fix the build issues and deal with the conflicts, I can release your changes as part of 5.8.0 and then in 5.8.1 we can enable mantine-ui in the playground

@pkalisiewicz
Copy link
Author

hey @heath-freenome I have updated my PR, locally everything builds fine.
I hope it should now build on CI fine as well

@heath-freenome
Copy link
Member

@pkalisiewicz It looks like the build is still having the Typescript issue.

@pkalisiewicz
Copy link
Author

@heath-freenome Sorry about that, I just realized I didn't add mantine it to the devDependencies, and it was getting the node modules from the root locally, but on the instance, it couldn't. That's solved!

@heath-freenome
Copy link
Member

@pkalisiewicz It still isn't working with the same typescript issue.

@heath-freenome
Copy link
Member

@pkalisiewicz Have you figured out why the builds keep failing with a typescript issue?

@eoin-obrien
Copy link

eoin-obrien commented Jun 5, 2023

Stumbled across this PR as I'd really like to use RJSF with Mantine in some of my company's projects!

I have a hunch that the error in the CI build is being caused by a versioning issue with Mantine. The devDependency calls for "@mantine/core": "^4.2.12".

semantic error TS2339: Property 'Wrapper' does not exist on type 'InputComponent'.

The input component docs for Mantine v5 and v6 both include Input.Wrapper. However, the docs for v4 show the InputWrapper component being imported separately rather than being a property of Input. Looks like breaking changes were made between v4 and v5.

I'd suspect that bumping the minimum required version of Mantine up to v5 for both peer and dev dependencies would resolve the TypeScript build issue. I'll test this out locally when I get a chance, but thought I'd note this here in case it's helpful!

@eoin-obrien
Copy link

I reproduced the TypeScript build error locally. Updating the minimum required version of Mantine to v5 resolves the issue and the build succeeds.

@heath-freenome
Copy link
Member

@pkalisiewicz Can you update the mantine version to v5 and try again?

@pkalisiewicz
Copy link
Author

@pkalisiewicz Can you update the mantine version to v5 and try again?

So sorry for the late response.
I have been extremely busy in the last few weeks.

I have updated it now

@heath-freenome
Copy link
Member

@pkalisiewicz two things, first please revert any of the package-lock.json files that are not related to mantine-ui or the playground. Second, there seems to be an issue with imports for the playground that I've seen before. I'm not sure about the fix at this point.

@dbsmck
Copy link

dbsmck commented Jul 13, 2023

Super interested in this!

@nachiket-p
Copy link

Hope to see this getting sorted soon!

@heath-freenome
Copy link
Member

@pkalisiewicz @dbsmck @nachiket-p We need someone to help get these changes over the line

@msjaber
Copy link

msjaber commented Oct 9, 2023

hey @pkalisiewicz, any chance you'd do this soon or someone should handle the remaining issues?

@heath-freenome
Copy link
Member

@msjaber If you'd like to take over please do so!


const emptyValue: string[] | string = multiple ? [] : '';
const dropdownOptions = createDefaultValueOptionsForDropDown<S>(enumOptions, enumDisabled);
const _onChange = (e: any) => onChange(enumOptionsValueForIndex<S>(e.value, enumOptions, emptyValue));

Choose a reason for hiding this comment

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

@pkalisiewicz

here now, you using e.value, yet the mantine <Select />'s props onChange will pass string and <MaltiSelect /> will pass string[]

So, you shoule rewrite such as

(e: string | string []) =>  ...

please check here : mantine Select
and here : mantie MultiSelect

@chriscors
Copy link

Any update on this? I'd love to see this go live and use it!

@yuki-js
Copy link
Contributor

yuki-js commented Jan 8, 2024

I am developing Mantine theme based on Mantine v7.
It came from my private project, so there might be some discrepancies to the original behaviors. I'm working for bringing them closer.

Though it can't be taken over this PR, due to incompatibility between react-frame-component in playground and Mantine v7, you can try it and also make a PR if someone fix the problem.
@pkalisiewicz thank you for your previous contribution!

https://github.com/AokiApp/rjsf-mantine-theme

@heath-freenome
Copy link
Member

@pkalisiewicz I hope you are ok, This PR is just waiting for you to finish it

@heath-freenome
Copy link
Member

I am developing Mantine theme based on Mantine v7. It came from my private project, so there might be some discrepancies to the original behaviors. I'm working for bringing them closer.

Though it can't be taken over this PR, due to incompatibility between react-frame-component in playground and Mantine v7, you can try it and also make a PR if someone fix the problem. @pkalisiewicz thank you for your previous contribution!

https://github.com/AokiApp/rjsf-mantine-theme

Which version of react-frame-component would work in the playground for your theme as well as the others?

@yuki-js
Copy link
Contributor

yuki-js commented Jan 30, 2024

@heath-freenome None would work as long as it relies on iframes. Mantine v7 has removed the Emotion dependency as well as the function to change the container into iframe like this. Without the function, iframe's CSS encapsulation would break. I could not fix it with v7, so I decided to fork it and make my own playground that don't use iframes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants