Skip to content

Conversation

@chrisgarrity
Copy link
Contributor

Resolves

Refactors query string handling into an HOC

Proposed Changes

Consolidates all the special handling of tutorial query strings into one place.

Reason for Changes

Easier to understand and maintain.

Test Coverage

Updated the unit tests to reflect the refactored tutorial URL handling. Also manually checked.

Browser Coverage

Check the OS/browser combinations tested (At least 2)

Mac

  • Chrome
  • Firefox
  • Safari

Windows

  • Chrome
  • Firefox
  • Edge

Chromebook

  • Chrome

iPad

  • Safari

Android Tablet

  • Chrome

@chrisgarrity
Copy link
Contributor Author

/cc @ericrosenbaum this will also handle the all tutorials

Copy link
Contributor

@benjiwheeler benjiwheeler left a comment

Choose a reason for hiding this comment

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

Looks great and works great!

Just one small thing I found to fix. And, it looks like you'll have to resolve conflicts (I guess with Paul's related commit, which could be tricky)

This will work better if there are other query string options that need to be handled later. It also handles the distinction between ‘all’ and the other tutorials better.
@benjiwheeler
Copy link
Contributor

LGTM

@chrisgarrity chrisgarrity merged commit 0ff1bd7 into scratchfoundation:develop Nov 19, 2018
@chrisgarrity chrisgarrity deleted the feature/refactor-all-tutorials branch November 19, 2018 20:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants