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 React components directory structure #2037

Merged
merged 52 commits into from
Dec 11, 2023
Merged

Conversation

EMaksy
Copy link
Member

@EMaksy EMaksy commented Nov 28, 2023

Description

This PR implements directory restructuring based on ADR 009. It separates the components directory into 'common' and 'pages,' enhancing code organization. The change ensures better reuse of common components across multiple pages.

Result after the refactor:
image

@EMaksy EMaksy added chore javascript Pull requests that update Javascript code labels Nov 28, 2023
@EMaksy EMaksy self-assigned this Nov 28, 2023
@EMaksy EMaksy force-pushed the refactor_components_dir branch 7 times, most recently from c184539 to 0f23efd Compare November 30, 2023 13:12
@EMaksy EMaksy changed the title Refactor React frontend directory structure Refactor React components directory structure Nov 30, 2023
@EMaksy EMaksy force-pushed the refactor_components_dir branch 3 times, most recently from 8e4de99 to 91132c6 Compare November 30, 2023 15:47
import { ClusterInfoBox } from '@components/ClusterDetails';
import HostInfoBox from '@components/HostDetails/HostInfoBox';
import { ClusterInfoBox } from '@pages/ClusterDetails';
import HostInfoBox from '@pages/HostSettingsPage/HostInfoBox';
Copy link
Contributor

Choose a reason for hiding this comment

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

I see here we borrow HostInfoBox from HostSettingsPage; going by our new structure, shouldn't we move HostInfoBox into the commmon/ directory then?

(Though this may not be for this PR)

Copy link
Contributor

Choose a reason for hiding this comment

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

(similar for ClusterInfoBox)

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 you're right @jamie-suse but I wouldn't follow through that in this PR 👍

Copy link
Member Author

Choose a reason for hiding this comment

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

I see there also opportunities to refactor some places, but i would not mind to split it up in a follow up pr :)

Copy link
Member Author

Choose a reason for hiding this comment

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

Moved like suggested 👍

Copy link
Contributor

@dottorblaster dottorblaster left a comment

Choose a reason for hiding this comment

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

I wouldn't follow through actual individual component refactoring through this PR but at the moment the state looks good.

@CDimonaco I would leave to you the final call about this 👍

Copy link
Contributor

@rtorrero rtorrero left a comment

Choose a reason for hiding this comment

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

Good stuff @EMaksy, I think this is a step in the right direction

@EMaksy EMaksy force-pushed the refactor_components_dir branch 2 times, most recently from 7538d51 to b23a8a9 Compare December 7, 2023 13:55
@EMaksy EMaksy merged commit 4155b2e into main Dec 11, 2023
26 checks passed
@EMaksy EMaksy deleted the refactor_components_dir branch December 11, 2023 10:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
chore javascript Pull requests that update Javascript code
Development

Successfully merging this pull request may close these issues.

None yet

5 participants