fix: reorganized components #562

arnaut08 · 2023-09-06T23:47:35Z

Issue number #546

Relevant #546

Resolves [DevEx]: Reorganize Vue components #546

Please check the following

Do the tests still pass? (see Run the Tests)
Is the code formatted properly? (see Linting (Formatting))
For New Features:
- Have tests been added to cover any new features or fixes?
- Has the documentation been updated accordingly?

Please describe additional details for testing this change

I don't think additional details are required as it's just the reorganization of the components files.

seriouslysean

Solid work so far! I think this is headed mostly in the right direction but I have a lot of thoughts on how I think we should be separating everything.

Considering how we utilize the components, it makes sense to me to split by route for now, and then organizing within route based folders.

Something like this:

/routes/
- /home/
  - /components
  - /js
- /about/
  - /components
  - /js

Etc, which would allow us to code split by route if we so choose by doing async imports. It also would keep a logical separation of components we use within those pages and we could set up route based aliases in vite. Any shared functionality would live under some sort of core, or global namespace.

@itsalaidbacklife penny for your thoughts.

arnaut08 · 2023-09-07T00:36:40Z

Considering how we utilize the components, it makes sense to me to split by route for now, and then organizing within route based folders.

Something like this:

/routes/

/home/

/components

/js

/about/

/components

/js

Currently we have all the views(route specific files) inside the /views directory. As per my understanding, you're suggesting for us to split the files as per the routes, right? So we'll have a /routes directory and individual directories for each route with respective view file, and respective components grouped in the components directory, right?
Something like this:

Main view file - /routes/home/HomeView.vue
Respective component files - /routes/home/components/...

Also, what exactly is the js subdirectory for? Is it for respective utility function files?

seriouslysean · 2023-09-07T11:03:46Z

Also, what exactly is the js subdirectory for? Is it for respective utility function files?

@arnaut08 exactly that. Utilities, store files, or anything other js imports related to just that route. You may not need the folder if the route doesn't have any right now, though.

I'll file a follow up to look in to async importing the top level Vue files from the router to garner the benefit from the file restructure.

See #563

arnaut08 · 2023-09-07T15:55:07Z

I've reorganized it as per the routes and handled #563 as well.
Please note: I couldn't find any route specific js files being imported anywhere and the utility files which are being imported in some routes is also being used in the server application. That's why I haven't included that in my changes yet. Please let me know if I missed anything.

seriouslysean

This is super close! Convert all the relative paths to aliases and in the vite config make an entry for the new areas; core, and each route as a top level so you can do something like ~core/components/FileName.Vue.

src/core/components/TheHeader.vue

src/routes/game/components/ChooseWhetherToCounterDialog.vue

src/routes/game/components/GameDialogs.vue

src/routes/game/components/GameUnavailableView.vue

itsalaidbacklife · 2023-09-14T13:17:23Z

@arnaut08 sorry to have left this hanging. This is looking good. I need to sync up with @seriouslysean and give this a look together but we should have the final feedback for you by end of week

arnaut08 · 2023-09-14T18:18:07Z

Sorry for the delayed response. I've made the changes suggested by @seriouslysean. Please review those changes and consider them for the feedback. Thank you.

itsalaidbacklife

This is a great start. I apologize that it's taken so long to get a full review on this. It's a high impact restructuring so we wanted to make sure the internal team was in sync about the desired structure. I'm requesting a slight rework of how the folders are organized, and of removing the ~ aliases and sticking with @ relative imports.

For the folder change, we will remove the core folder, and move both core/components and core/composables directly into src/components (for base components) and src/composables (for root composables).

Additionally, let's rename the src/routes/auth folder to src/routes/login as that's the name of the route.

Again, great work! This will add a lot of value, especially making it easier for new contributors to find components while onboarding. Thanks for your contribution! Please comment here (or join us on discord) if you have any questions.

src/core/components/AwardCard.vue

src/core/components/RulesDialog.vue

src/core/components/UsernameToolTip.vue

src/core/composables/navLink.js

src/router.js

vite.config.js

src/routes/game/components/SevenDoubleJacksDialog.vue

itsalaidbacklife · 2023-10-09T17:50:05Z

@arnaut08 are you still interested in finishing this task?

arnaut08 · 2023-10-09T17:56:04Z

@arnaut08 are you still interested in finishing this task?

Hey! I'm so sorry for the delayed response. Yes, I'll make the changes as per your latest comments.

itsalaidbacklife · 2023-10-09T18:26:17Z

@arnaut08 it's not a problem at all. Open source is always at your own pace, and these kinds of sweeping reorganization bear the brunt of the challenge around merge conflicts.

I'm just commenting on the open PR's to check which ones are abandoned and should be closed accordingly. I'll leave this open. Reach out here or on discord if you get stuck or would like help.

…e/546

itsalaidbacklife

@arnaut08 excellent work. This all looks right. I have two whitespace nitpicks in the comments. Other than that, the only thing to do is to fix the merge conflicts. Let me know if you'd like help navigating them.

This is a great reorganization that I'm looking forward to finalizing. Thanks for your contribution!

src/routes/home/components/GameListItem.vue

src/stores/gameList.js

…e/546

itsalaidbacklife · 2023-10-11T17:10:13Z

@arnaut08 this is great. There are just two remaining merge conflicts in

src/components/HowItWorksDialog.vue
src/views/RulesView.vue

which were just updated today. Apologies that this has been a moving target. I can stop merging other PR's until this is done so that your update can go live before other things go in because you've been working on this for a while and it is so high-impact

…e/546

arnaut08 · 2023-10-11T19:12:57Z

Apologies that this has been a moving target

No worries at all. I've resolved the remaining conflicts.

itsalaidbacklife

Great work, thanks for taking this on!

Change requests are addressed

* fix: reorganized components * fix: reorganized files as per the routes * fix: aliases added and routes reconfigured * fix: reconfigured paths * fix: corrected paths after resolving conflicts * fix: removed extra whitespaces * fix: fixed remaining paths

fix: reorganized components

1a9cc4d

arnaut08 requested review from itsalaidbacklife and seriouslysean as code owners September 6, 2023 23:47

seriouslysean requested changes Sep 7, 2023

View reviewed changes

fix: reorganized files as per the routes

a9c9e23

arnaut08 requested a review from seriouslysean September 7, 2023 15:56

seriouslysean previously requested changes Sep 7, 2023

View reviewed changes

fix: aliases added and routes reconfigured

64754bb

arnaut08 requested a review from seriouslysean September 14, 2023 18:18

itsalaidbacklife reviewed Sep 25, 2023

View reviewed changes

arnaut08 added 4 commits October 10, 2023 21:34

fix: reconfigured paths

a1ac4fa

Merge branch 'main' of https://github.com/arnaut08/cuttle into featur…

c29ff06

…e/546

fix: corrected paths after resolving conflicts

c8e23a8

Merge branch 'main' of https://github.com/arnaut08/cuttle into featur…

5db3dee

…e/546

arnaut08 requested a review from itsalaidbacklife October 10, 2023 18:31

itsalaidbacklife reviewed Oct 11, 2023

View reviewed changes

src/routes/home/components/GameListItem.vue Outdated Show resolved Hide resolved

src/stores/gameList.js Outdated Show resolved Hide resolved

arnaut08 added 3 commits October 11, 2023 19:35

fix: removed extra whitespaces

4f83916

Merge branch 'main' of https://github.com/arnaut08/cuttle into featur…

0dbb5f9

…e/546

fix: fixed remaining paths

251fbbf

itsalaidbacklife added the hacktoberfest This issue welcomes contributions for Hacktoberfest. label Oct 11, 2023

arnaut08 requested a review from itsalaidbacklife October 11, 2023 15:19

Merge branch 'main' of https://github.com/arnaut08/cuttle into featur…

71e003d

…e/546

itsalaidbacklife approved these changes Oct 11, 2023

View reviewed changes

itsalaidbacklife merged commit e7245f6 into cuttle-cards:main Oct 11, 2023
9 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: reorganized components #562

fix: reorganized components #562

arnaut08 commented Sep 6, 2023

seriouslysean left a comment •

edited

Loading

arnaut08 commented Sep 7, 2023 •

edited

Loading

seriouslysean commented Sep 7, 2023 •

edited

Loading

arnaut08 commented Sep 7, 2023

seriouslysean left a comment

itsalaidbacklife commented Sep 14, 2023

arnaut08 commented Sep 14, 2023

itsalaidbacklife left a comment

itsalaidbacklife commented Oct 9, 2023

arnaut08 commented Oct 9, 2023

itsalaidbacklife commented Oct 9, 2023

itsalaidbacklife left a comment

itsalaidbacklife commented Oct 11, 2023 •

edited

Loading

arnaut08 commented Oct 11, 2023

itsalaidbacklife left a comment

fix: reorganized components #562

fix: reorganized components #562

Conversation

arnaut08 commented Sep 6, 2023

Issue number #546

Please check the following

Please describe additional details for testing this change

seriouslysean left a comment • edited Loading

Choose a reason for hiding this comment

arnaut08 commented Sep 7, 2023 • edited Loading

seriouslysean commented Sep 7, 2023 • edited Loading

arnaut08 commented Sep 7, 2023

seriouslysean left a comment

Choose a reason for hiding this comment

itsalaidbacklife commented Sep 14, 2023

arnaut08 commented Sep 14, 2023

itsalaidbacklife left a comment

Choose a reason for hiding this comment

itsalaidbacklife commented Oct 9, 2023

arnaut08 commented Oct 9, 2023

itsalaidbacklife commented Oct 9, 2023

itsalaidbacklife left a comment

Choose a reason for hiding this comment

itsalaidbacklife commented Oct 11, 2023 • edited Loading

arnaut08 commented Oct 11, 2023

itsalaidbacklife left a comment

Choose a reason for hiding this comment

seriouslysean left a comment •

edited

Loading

arnaut08 commented Sep 7, 2023 •

edited

Loading

seriouslysean commented Sep 7, 2023 •

edited

Loading

itsalaidbacklife commented Oct 11, 2023 •

edited

Loading