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

[2.x] Move Jetstream components to Components directory #1110

Merged
merged 1 commit into from
Aug 8, 2022

Conversation

jessarcher
Copy link
Member

This PR renames the resources/js/Jetstream directory to resources/js/Components.

This signals to the user that the components belong to them and can be used outside the Jetstream pages and customized as needed. I've seen repos where people create their own Components directory and maintain two sets of buttons, etc.

This is also consistent with Breeze.

The intention with this change is to signal to users that the components
are theirs for customizing.
@jessarcher jessarcher changed the title Move Jetstream components to Components directory [2.x] Move Jetstream components to Components directory Aug 8, 2022
@taylorotwell taylorotwell merged commit 80641e2 into 2.x Aug 8, 2022
@taylorotwell taylorotwell deleted the move-jetstream-components branch August 8, 2022 13:25
@joelbutcher
Copy link
Contributor

@jessarcher Would it also be worth renaming the imports of these components to equally detach the perceived ownership from Jetstream? e.g import JetButton from '@/components/Button.vue to import Button from '@/components/Button.Vue?

@jessarcher
Copy link
Member Author

@joelbutcher I'd love to, but the Vue style guide says not to.

Yesterday I asked Evan You whether he'd still advocate against it and this was his response:

Not as strictly, but I’d still avoid names that overlap with known native elements.

So for components like Modal I think we should be fine. For things like Button, maybe we figure out some more descriptive prefixes for them. E.g. PrimaryButton, SecondaryButton. Input and Label might be TextInput and InputLabel or something.

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.

5 participants