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

Refactor chakra components into separate folder #2315

Merged
merged 16 commits into from Dec 21, 2023

Conversation

picklelo
Copy link
Contributor

This change moves most of the core components into the reflex/components/chakra namespace, in order to prepare for the switch to the Radix components.

All imports for end-users should work exactly as is, this is just an internal restructuring.

Copy link
Collaborator

@Lendemor Lendemor left a comment

Choose a reason for hiding this comment

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

A few changes needed but I'm glad this PR is finally here 😄

Copy link
Collaborator

Choose a reason for hiding this comment

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

To keep with how we do it for radix, maybe this chakra.py file need to be renamed base.py since it's already inside the chakra folder anyway

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good call

from .media.image import Image

image = Image.create
from .chakra import *
Copy link
Collaborator

Choose a reason for hiding this comment

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

from .base import * if we actually rename chakra.py as mentioned in other comment.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Debounce is not chakra specific, it can probably go to core or it's own folder.

Copy link
Collaborator

Choose a reason for hiding this comment

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

we actually use this for the radix textfield as well

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Moving this

Copy link
Collaborator

Choose a reason for hiding this comment

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

since Password doesn't actually exist in Chakra, this should probably become a high level component only, that specialize an input with type="password"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Agreed, I think these ones are tied to the Chakra Input component, maybe we can leave it for now and abstract it later.

reflex/components/chakra/layout/fragment.pyi Outdated Show resolved Hide resolved
reflex/components/chakra/navigation/nextlink.pyi Outdated Show resolved Hide resolved
reflex/components/chakra/overlay/banner.pyi Outdated Show resolved Hide resolved
reflex/components/radix/themes/components/textarea.pyi Outdated Show resolved Hide resolved
reflex/app.pyi Outdated
@@ -12,7 +12,7 @@ from reflex.components.component import (
Component as Component,
ComponentStyle as ComponentStyle,
)
from reflex.components.layout.fragment import Fragment as Fragment
from reflex.components.chakra.layout.fragment import Fragment as Fragment
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
from reflex.components.chakra.layout.fragment import Fragment as Fragment
from reflex.components.base.fragment import Fragment as Fragment

i thought this got moved to base

Copy link
Collaborator

Choose a reason for hiding this comment

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

we actually use this for the radix textfield as well

reflex/components/chakra/forms/upload.pyi Outdated Show resolved Hide resolved
requirements.txt Outdated Show resolved Hide resolved
masenf
masenf previously approved these changes Dec 20, 2023
Copy link
Collaborator

Choose a reason for hiding this comment

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

why no pyi file for this component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure, I ran the generator

Copy link
Collaborator

Choose a reason for hiding this comment

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

multiselect.py is black-listed in the generator, probably a remnant of when it was broken ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We can bring it back now I suppose

from .gridjs import *
from .markdown import *
from .moment import *
from .next import NextLink, next_link
Copy link
Collaborator

Choose a reason for hiding this comment

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

With this change do we still have access to the namespaced rx.next.image and rx.next.video ?

@picklelo picklelo merged commit 93c9738 into main Dec 21, 2023
45 checks passed
@picklelo picklelo deleted the nikhil/refactor-components branch January 30, 2024 03:09
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

3 participants