Skip to content

Feature/open tutorial page in new tab and move playback buttons to left#63

Merged
schoinh merged 4 commits intomasterfrom
feature/open-tutorial-in-new-tab
Nov 5, 2020
Merged

Feature/open tutorial page in new tab and move playback buttons to left#63
schoinh merged 4 commits intomasterfrom
feature/open-tutorial-in-new-tab

Conversation

@schoinh
Copy link
Copy Markdown
Contributor

@schoinh schoinh commented Nov 4, 2020

Resolves: open tutorial in new tab
AND Move all the playback buttons to the left of the slider

To use the useLocation React Router hook, I had to make a new separate function component to house the Getting Started link in the header. All ears if someone has a better way. Just using the built-in location.pathname without using useLocation didn't work -- if I went to the viewer, then came back to the landing page using the Simularium Home link, then clicked on Getting Started in the header, then the tutorial page would open in a new tab.

Pull request recommendations:

  • Name your pull request your-development-type/short-description. Ex: feature/read-tiff-files
  • Link to any relevant issue in the PR description. Ex: Resolves [gh-##], adds tiff file format support
  • Provide description and context of changes.
  • Provide relevant tests for your feature or bug fix.
  • Provide or update documentation for any feature added by your pull request.

Thanks for contributing!

@schoinh schoinh requested review from a user, blairlyons, meganrm and toloudis November 4, 2020 00:02
@schoinh schoinh changed the title Feature/open tutorial page in new tab Feature/open tutorial page in new tab and move playback buttons to left Nov 4, 2020
Comment thread src/components/HeaderFooter/index.tsx Outdated
import { useLocation, NavLink } from "react-router-dom";
import { TUTORIAL_PATHNAME } from "../../routes";

const HeaderFooter: React.FunctionComponent<{}> = () => {
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Small thing, but I think it would be clearer to call this "TutorialLink" or something like that. HeaderFooter is a little confusing

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Hah, yeah I admit it's a bad name. I'm going to change it to HeaderDropdowns because it will soon contain both the "Load model" and "Support" dropdowns.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Actually I'll call it TutorialLink in this PR so it reflects its current state and then change it to HeaderDropdowns for my future PR

Copy link
Copy Markdown
Contributor

@meganrm meganrm left a comment

Choose a reason for hiding this comment

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

Looks good to me, just one component name change suggestion

@schoinh schoinh merged commit e5741a4 into master Nov 5, 2020
@schoinh schoinh deleted the feature/open-tutorial-in-new-tab branch November 5, 2020 18:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants