Skip to content
This repository has been archived by the owner on May 24, 2024. It is now read-only.

react-router-url #26

Merged
merged 1 commit into from
Jun 21, 2017
Merged

react-router-url #26

merged 1 commit into from
Jun 21, 2017

Conversation

Matt-Butler
Copy link
Contributor

Summary

This fixes the react-router URLs. Currently, if you go to https://terra-ui.herokuapp.com/components/core/badge, the route does not exist. This is because this route is generated by react-router. This pr fixes that issue.

This fix is the same thing react_on_rails suggests.
https://github.com/shakacode/react-webpack-rails-tutorial/blob/master/config/routes.rb#L13

Thanks for contributing to Terra.
@cerner/terra

@windse7en
Copy link
Contributor

I found a weird thing on the header id jump. For example:
If I want to hit http://localhost:3000/components/core/modal#isOpened directly, it will still jump to the top of http://localhost:3000/components/core/modal page. The header id doesn't work in here.

But if I hit http://localhost:3000/components/core/modal at first, and try to hit http://localhost:3000/components/core/modal#isOpened. It jumps to the header with isOpened id, works as expected.

@Matt-Butler
Copy link
Contributor Author

Matt-Butler commented Jun 19, 2017

I found a weird thing on the header id jump. For example:
If I want to hit http://localhost:3000/components/core/modal#isOpened directly, it will still jump to the top of http://localhost:3000/components/core/modal page. The header id doesn't work in here.

But if I hit http://localhost:3000/components/core/modal at first, and try to hit http://localhost:3000/components/core/modal#isOpened. It jumps to the header with isOpened id, works as expected.

It looks like this issue has been resolved in react-router v4.
remix-run/react-router#394 (comment)

What do you think about logging this as a seperate issue and fix it when we upgrade to react-router v4?

@bjankord
Copy link
Contributor

I'm fine with logging an issue for when we update RR to v4. Before that can we test the sample code from that issue link for RR v3 for creating a hashLinkScroll function and passing that to the Route onUpdate prop .

import React from 'react';
import { render } from 'react-dom';
import { Router, Route, browserHistory } from 'react-router';

const routes = (
  // your routes
);

function hashLinkScroll() {
  const { hash } = window.location;
  if (hash !== '') {
    // Push onto callback queue so it runs after the DOM is updated,
    // this is required when navigating from a different page so that
    // the element is rendered on the page before trying to getElementById.
    setTimeout(() => {
      const id = hash.replace('#', '');
      const element = document.getElementById(id);
      if (element) element.scrollIntoView();
    }, 0);
  }
}

render(
  <Router
    history={browserHistory}
    routes={routes}
    onUpdate={hashLinkScroll}
  />,
  document.getElementById('root')
)

@windse7en
Copy link
Contributor

windse7en commented Jun 20, 2017

+1 to add hashLinkScroll() functions. # scroll should be a quite common use case, and it will we better if we can support that before upgrading react-router to v4. I use it a lot.

All other things look good to me.

@Matt-Butler
Copy link
Contributor Author

The function they provided here did not fix the issue.

I logged this issue to fix the hash links: #29

@Matt-Butler Matt-Butler merged commit 93118d7 into master Jun 21, 2017
@bjankord bjankord deleted the react-router-url branch January 17, 2019 15:13
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants