Layout/datasets page

[NanaKay007]

@rileyjshaw First of all, Thanks for taking your time to review my code :) I've worked on this page for a while, and would love some feedback concerning coding style, etc.

In demo-layout.tsx, I specify a list of routes that render the same component: Kiosk (defined in kiosk.tsx). The Kiosk component is a wrapper that defines styles for PageContent.Card and PageContent.Content components, both defined by a PageContentFactory depending on which route has been visited (the /demo/datasets and /demo/dashboard routes are handled for now).

I created two types.ts: one in pages/datasets and kiosk/. The former defines interfaces that are shared by components in pages/datasets/datasets-info.tsx and kiosk/dataset.tsx files. The latter defines types specific to kiosk components such as those in kiosk/dataset.tsx.

I am not entirely sure what the purpose of the 'Browse Languages' selector (in the Card body for the datasets page on the figma spec) is. Please let me know if there are any details concerning that. I haven't tackled page reponsiveness yet (awaiting feedback from the JZ about that).

[rileyjshaw]

Hi @NanaKay007, thanks for the pull request! (and sorry for the delay, I was out on Friday)

I'm excited to jump into this. First, I want to clarify which change I should look at. There seem to be some unrelated commits bundled into this PR; I think the mozilla:gsoc branch hadn't been updated in a while, so NanaKay007:layout/datasets-page has some extra commits from mozilla:master that aren't on the gsoc branch yet.

I just merged master into mozilla:gsoc, but I don't have push access to your NanaKay007 fork. Before I can review this, can you please ensure your branch is up to date with mozilla:gsoc by merging or rebasing?

If it's helpful, this was my attempt at fixing merge conflicts between your branch and mozilla:gsoc: https://gist.github.com/rileyjshaw/a3306c59b7a1afc08a111282e8ccd77f. At time of writing only two files had conflicts that needed resolving 🤞

[NanaKay007]

@rileyjshaw

Hi Riley. Thanks for getting back to me. I apologize for the numerous commits. The commits that need review are from 4 days ago. I have updated the pr with the changes from gsoc and resolved conflicts. I hope it fixes things. thank you.

EDIT: do you think creating a new PR will fix the issue with Travis-ci checks?

[rileyjshaw]

@NanaKay007, thanks for doing that! I'm still seeing 52 commits in this pull request. You can see them listed here: https://github.com/mozilla/voice-web/pull/2768/commits.

I looked into it, and it appears that the mozilla:gsoc branch only has two commits from you:

• layout/demo-routes
• layout/intro-page

these appear to be squashed versions of the extra commits I'm seeing here. I'm not positive that this will work, but can you please try rebasing your branch onto mozilla:gsoc and then force pushing it here? If you get any CONFLICT warnings, you can always run git rebase --abort.

git branch layout/datasets-page-backup
git remote add upstream git@github.com:mozilla/voice-web.git
git fetch upstream
git rebase upstream/gsoc
# If all goes well…
git push -f

[NanaKay007]

@rileyjshaw
rebasing didn't work as expected. I tried resolving the conflicts manually, but I had to create new commits afterwards since some commits caused code errors. Should I move ahead with that or is there something else I could do?

[rileyjshaw]

@NanaKay007 thanks for sticking with this, I really appreciate it!

Another option that you could try is:

git checkout --track upstream/gsoc
git pull
git checkout -b layout/datasets-page-r2
# Replace "A" below with the first commit hash you want in the PR
# Replace "B" below with the last commit hash you want in the PR
git cherry-pick A..B
git push -fu origin layout/datasets-page

The above basically starts you from scratch, then cherry-picks the commits you want reviewed.

---

The underlying issue here (and one that will be good to know for future PRs) is that when you finally merge a PR into the upstream repository, all the commits in that PR are squashed into a single commit. When that happens, your local fork is now out of date with the mozilla:gsoc branch ("upstream"), and has a conflicting history. By squashing many commits into one, we've "rewritten history", and it can lead to confusing situations like this.

In the future, when starting out on a new feature it's usually easiest to start from the upstream version.

---

I know this is a lot, please let me know if I can explain anything better.

[NanaKay007]

@rileyjshaw
I doubt that I got everything in perfect shape but I hope this makes things slightly better. Really appreciate your support.

[phirework]

Hey @NanaKay007, it looks like the latest squash actually undid a bunch of the changes you made in previous PRs - if you look at the files changed tab you've got a lot of reverted class names and the router is checking for a hardcoded value again. Let's debug this live during our check-in tomorrow, and then I think it'll probably make more sense to open a new PR since this one has a lot of messy history on it already.