Reorganize front-end files, to have a dedicated pages folder

[tomalec]

This is an issue to continue a discussion on pages folder started at https://github.com/woocommerce/google-listings-and-ads/pull/192#issuecomment-773084286

@eason9487 wrote:

Since we did not create a dedicated folder for pages, we can still have the alias for the js/src directory. And add a pages alias if we adjust the file structure for putting these pages together in the future.

@ecgan wrote:

Yeah I agree that we should have a dedicated folder for pages, it would be easier to tell which ones are the entry point / components at the highest level. 😄

---

Personally, I totally upvote this idea as well.

I think we can have a folder for pages that matches the navigation, like:

/js/src/
├── components/
├── css/
├── hooks/
├── pages/
│   ├── getting-started
│   ├── dashboard
│   ├── reports
│   │   ├── programs
│   │   └── products
│   ├── product-feed
│   └── settings
├── utils/
├── tasks/
└── wcdl/

It would help to:

• identify highest-level components and a relationship between individual components
• make folders smaller

Doubts

• Should we put setup-* pages inside "Getting started", as they do not have their own menu entry after the plugin is set up?
• Shouldn't we have hooks per-component, or move them to utils?

Additional comments

I think it would be good to move all the files with one PR, instead of adapting gradually.

//cc @haszari maybe you would like to comment on that as well.

[ecgan]

I think that the pages folder structure should match those in https://github.com/woocommerce/google-listings-and-ads/blob/trunk/js/src/index.js: in that file, we can see the relationship between path and container, and the container is what I regard as "page component", "entry point component", "top level component".

Should we put setup-* pages inside "Getting started", as they do not have their own menu entry after the plugin is set up?

So based on my above point, yes we should put them as /pages/setup-mc and /pages/setup-ads etc.

Shouldn't we have hooks per-component, or move them to utils?

I usually prefer code colocation - put functions (hooks, utils, anything) close to where it is being used. If it is being used in one component only, put it side-by-side in one folder. If it is used by multiple components, move it up the folder hierarchy tree.

To me, hooks are a special breed of functions and deserve to be separated from utils. Hooks can call util, but util shouldn't call hooks. Hooks can only be called by a functional component or another hook.

[haszari]

@haszari maybe you would like to comment on that as well.

In general this looks like a good idea 🚀. I'll leave you wooglers to discuss the details and how/when to implement any changes to folder structure 😄

[tomalec]

Seems our discussion and agreement came stale to the point where we would like to:

1. have dedicated /pages folder
2. have the folder structure be consistent with what we have in /js/src/index.js
3. keep code possibly colocated - if util, hook, or any other dependency is used by a single component - keep it close to the component; if it's shared - bubble it up the tree.
4. keep hooks in dedicated files separated from other utils

---

Side note:

I'm not 100% sure that the structure and paths in /js/src/index.js are perfect, and we could fiddle a little bit there. For example, the initial MC setup falls into the same category/bag/folder as the other onboarding pages like getting started, but that's something we could consider a separate issue from the general idea of separating pages. I definitely see the value of consistency between folder tree structure and index.js and think it's a good idea to make /pages follow what we already have there.

---

Unless somebody disagrees, I'd be happy to take an action for the first step to change our current /js/src/

├── components/
├── css/
├── dashboard/
├── data/
├── get-started/
├── hello/
├── hooks/
├── product-feed/
├── reports
│   ├── programs
│   └── products
├── settings
├── setup-ads
├── setup-mc
├── tab-nav/
├── tasks/complete-setup/
├── utils/
├── wcdl/
└── index.js

to

├── components/
│   ├── ...
│   └── tab-nav/
├── css/
├── data/
├── hooks/
├── pages/
│   ├── getting-started/
│   ├── dashboard/
│   ├── reports/
│   │   ├── programs/
│   │   └── products/
│   ├── product-feed/
│   ├── settings/
│   ├── setup-ads/
│   └── setup-mc/
├── utils/
├── tasks/complete-setup/
├── wcdl/
└── index.js

removed `hello/` as not used

---

I have a problem with tab-nav. On one hand, it's just a component, not a page, but on the other, it's so tied to the structure of our pages, it's more like another navigation setup as the /js/src/index.js.

With new navigation, the problem will be gone here but will remain in reports/.

In my mental model, the thing that brings an instance of tab navigation with all tab labels & co. is the "entry point"/"top-level component" that then routes the given path to individual sub-page components. However, in our implementation, it works the other way around - the individual page employs tab navigation with all possible "sibling" pages. That makes it hard for me to decide whether "TabNav" is a page/component/navigation configuration. @ecgan what's your take on that?

[jconroy]

I think things have evolved enough that it's a ~good~ great change.

If it's something that will take 30-60min, I'd say sure.

But if it's going to take a day or more of refactoring away from finishing the onboarding/setup flows I can live it with as is. We need to get these flows testable by users as soon as possible.

[tomalec]

I think it will take 30-60mins, but I'd wait for MS Setup PRs (https://github.com/woocommerce/google-listings-and-ads/pull/217, https://github.com/woocommerce/google-listings-and-ads/pull/212) to be merged, to avoid merging problems.

[jconroy]

Gotcha, sounds good to me

[ecgan]

@tomalec ,

Unless somebody disagrees, I'd be happy to take an action for the first step to change our current /js/src/

│   ├── reports/
│   │   ├── programs/
│   │   └── products/

I'd suggest to make it a flat structure instead of nested inside reports folder, like this:

│   ├── programs-report
│   ├── products-report

The idea is that the containers that we see in js/src/index.js correspond to the folders in pages folder.

// in js/src/index.js:
container: ProgramsReport,    // based on this - "ProgramsReport" is the entry point component or page component. 
path: '/google/reports/programs',    // not this. Path can be easily changed and it is possible to have multiple paths rendering same container page component.

---

I have a problem with tab-nav. [...] @ecgan what's your take on that?

I briefly mentioned some problems with the tab-nav back in Nov 2020 in https://github.com/woocommerce/google-listings-and-ads/pull/74#issuecomment-734921343 .

A better way to do the tab navigation is have a rendering structure like this (all names below are just examples):

Entry component
-> TabNav component (check URL path query and display the appropriate child tab content)
--> ChildTabContent component ("dashboard")
--> ChildTabContent component ("reports")
----> ChildTabReportsContent component ("programs report")
----> ChildTabReportsContent component ("products report")
--> ChildTabContent component ("product feed")
--> ChildTabContent component ("settings")

This means that there is only one TabNav component instance instead of multiple TabNav component instances that we have now in the code. With this one TabNav component, it would solve the problems I mentioned in https://github.com/woocommerce/google-listings-and-ads/pull/74#issuecomment-734921343.

This also means that some of those paths in the js/src/index.js would be rendering the same Entry component:

container: EntryComponent,
path: '/google/product-feed',
...

container: EntryComponent,
path: '/google/settings',
...

In essence, I think my idea above is the same as your mental model:

In my mental model, the thing that brings an instance of tab navigation with all tab labels & co. is the "entry point"/"top-level component" that then routes the given path to individual sub-page components.

Let me know what you think. Happy to discuss more 🙂

[eason9487]

@puntope wrote in https://github.com/woocommerce/google-listings-and-ads/pull/1374#issuecomment-1084429913:

💅 Not related to this PR but after seeing your nice work in refactoring the cards to the components I was wondering if it would be even nicer to organize the components folder.

• Cards -> All the cards
• Icons -> Icons
• Controls -> Inputs / user interaction elements like inputs, selects, etc
• Rest in the root...

I think it's also a good thought for this follow-up issue.