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: split out ui file #3865

Merged
merged 10 commits into from
Oct 30, 2024
Merged

refactor: split out ui file #3865

merged 10 commits into from
Oct 30, 2024

Conversation

jamdelion
Copy link
Contributor

@jamdelion jamdelion commented Oct 28, 2024

Sorry for the long diff - most of these are file import changes.

Summary of main changes:

The ui.tsx file seemed very random - filled with shared components such as MoreInformation, miscellaneous shared types (e.g. PublicProps) and the long icons mapping constant.

  • I have split this into shared/icons, shared/types.tsx and moved the components into their own files, which are under /src/ui/editor.
  • Also I've split out types.tsx from Checklist.tsx

@jamdelion jamdelion marked this pull request as ready for review October 28, 2024 18:06
@jamdelion jamdelion requested a review from a team October 28, 2024 18:06
Copy link

github-actions bot commented Oct 28, 2024

Removed vultr server and associated DNS entries

Copy link
Contributor

@DafyddLlyr DafyddLlyr left a comment

Choose a reason for hiding this comment

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

Great little refactor, love to see it!

A few comments on co-locating the Editor modal components 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

I think this should move to /src/ui/editor/ to be located alongside other Editor modal components.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, that makes sense! I think I've got a few holes in my understanding of the context around these groupings so that's helpful to have that framing :)

Copy link
Contributor

Choose a reason for hiding this comment

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

As above, this should go in /src/ui/editor.

The intention here would be for src/@planx/components to be the graph components. There's a tricky language thing here where these are the lego building blocks of the graph, but the word "component" in the lego block sense clashes with "component" in the web framework sense.

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 wonder if it would make sense to rename one of the folders to graphComponents in that case? 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

Just discussed in dev call - let's stick with /components for now 👍

Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe /shared/types.tsx would be more appropriate here?

@jamdelion jamdelion merged commit 27a796c into main Oct 30, 2024
12 checks passed
@jamdelion jamdelion deleted the jh/refactor-ui-file branch October 30, 2024 16:53
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.

2 participants