Migrates pages module

[edvardmk]

Proposed Changes

• creates react components for pages:
  -- /donate
  -- /donate/help
  -- /donate/policy
  -- /faq
  -- /faq/foundation
  -- /faq/technlogy
  -- /faq/tribes
  -- /foundation
  -- /guide
  -- /home
  -- /media
  -- /navigation
  -- /privacy
  -- /rules
  -- /team

• angular views for these pages

• changes pages.client.routes.js

Testing Instructions

• npm install
• npm start
• the above listed pages

Fixes #

[mrkvon]

Wow, this is a lot of work done!

I've added some comments on appearance of pages from /donate till /foundation. To be continued.

I've also noticed one or two minor issues with Board component, however that's unrelated to this PR.

Also the client tests are failing. I suspect it's related to adding $broadcast to Board component. Maybe this function will need mocking in tests. That's a guess. I haven't really investigated it.

[edvardmk]

Thanks a lot for this valuable input @mrkvon ! I was looking out for all these things but missed this bunch in the end. How should I proceed? Make the changes and force push the branch again?

The only thing I don't know how to solve is to make the tests work and what to do about the $broadcast in the Boards component you suggested.

[mrkvon]

@edvardmk

Make the changes and force push the branch again?

Yes, when you push changes, this pull request will be updated. You probably don't need --force when you add a new commit. Only if you change the already pushed commits. (e.g. after git rebase, git commit --amend or squashing commits).

make the tests work and what to do about the $broadcast in the Boards component

I'll have a look when I continue the review (evening or tomorrow) and see if i can help. There are many changes in this PR, so it will take me a while to review. Or maybe somebody else will jump in so we have it reviewed quicker. 🙂

[mrkvon]

Also, notice the changes in #1405 to fix conflicts with master. (git rebase master or git merge master). In case you need, I can help with that.

[nicksellen]

Also the client tests are failing. I suspect it's related to adding $broadcast to Board component. Maybe this function will need mocking in tests. That's a guess. I haven't really investigated it.

Indeed, if you just want to make the test work, then you can add this to the test file:

jest.mock('@/modules/core/client/services/angular-compat');

If you also want to assert that the $broadcast function was actually called as expected, you can do something like this:

import { $broadcast } from '@/modules/core/client/services/angular-compat';

jest.mock('@/modules/core/client/services/angular-compat');

// then, later inside a test block
expect($broadcast).toHaveBeenCalledWith({ whatever: 'it gets', called: 'with' });

See

trustroots/modules/messages/tests/client/components/Inbox.component.tests.js

Line 15 in 6bdf0f9

jest.mock('@/modules/core/client/services/angular-compat');

for an example of this using the eventTrack angular-compat function.

[nicksellen]

Something seems a bit up with the photo credits:

• I visit http://localhost:3000/faq/tribes
• I see a nice photo, and a photo credit at the bottom, all good
• I click on "Technology" (for example, but I don't think matters which one)
• I see a nice new photo, but there is no photo credit any more
• I do a full page reload of http://localhost:3000/faq/technology
• there is a photo + credit again

So, it's something around when the pages change after first load does not work. Maybe try tracing through what happens and what is fired (I use console.log() statements for this kind of stuff, but explore fancier debugger options if you want).

One thing I did notice is that the useEffect block in the board Board needs to declare names as a dependency [1], I think this was already like this in master maybe. Not sure if this is related to the issue.

[1] quick recap of useEffect "dependencies":

useEffect(() => {
}, []) // <--- this array is the "dependencies" (controlling when it will re-run the block)

// so if we use a prop

function MyComponent({ someProp }) {
  useEffect(() => {
     // does something with the prop...
     fetchThings(someProp);
  }, [someProp]); // <--- declaring the dependency on "someProp"
}

(I think https://reactjs.org/docs/hooks-overview.html is a nice overview of how hooks work, not currently loading for me though... 520 error...)

[nicksellen]

Great work! This is a significant effort, and nearly ready I think! I think a lot of learning went into this.

I check every page and didn't see anything wrong (apart from photo credits behaviour described above). I didn't click all the links but assume they are fine.

I'm not quite sure what the /navigation page is. I didn't look into it, have to eat now... looked a bit like a thing that is included into another thing, do we need it in react yet? (Or rather, is it used?). I probably couldn't have answer that myself in the time it took to write this paragraph.

[simison]

I'm not quite sure what the /navigation page is.

It's linked from mobile header, 3 lines link:

https://user-images.githubusercontent.com/87168/79903592-04a38f00-841c-11ea-9794-6b807997cf15.jpg

I think it'll need to be changed to "slide out" menu but it's useful to turn it into react already now.

[nicksellen]

After chatting with @edvardmk we decided it might be better if I fixed up the remaining issues, they were:

1. the broadcast calls were being made far too much, due to useEffect dependency using array value, fixed in 3aa531c
2. vm.photoCreditsCount was ended up as 0 even when there were some, causing the photos credits not to be displayed (this is actually a bug on existing site, not just on this branch as I had thought in Migrates pages module #1412 (comment)) fixed in 03f7aa3
3. the subscribe/unsubscribe to $on events with useEffect wasn't quite right, fixed in 0b16cb7
4. test failures because we use angular-compat now, which needs to be mocked with jest, as described in Migrates pages module #1412 (comment), fixed in dae7fff (also added a basic test in fd0cef8)

The commits themselves have reasonable detailed messages, and I also removed a few unneeded comments/TODO/files...

Also, maybe out of interest my main "debugging" method to work this out, was just loads of dumb console.log calls in various places to see what is happening...