Skip to content

accessing statics of activeRoute #107

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
jlongster opened this issue Jul 23, 2014 · 30 comments
Closed

accessing statics of activeRoute #107

jlongster opened this issue Jul 23, 2014 · 30 comments

Comments

@jlongster
Copy link
Contributor

Current activeRoute is a wrapper function that creats the child component:

    childHandler = function (props, addedProps) {
      var children = Array.prototype.slice.call(arguments, 2);
      return route.props.handler.apply(null, [mergeProperties(props, addedProps)].concat(children));
    }.bind(this, props);

This makes it so that I can't access the actual component directly, so I can't access the any static properties defined with statics. Right now my use case is to specify a title of the page, and the outer app handler will get it and set the title.

var Page = React.createClass({
  statics: {
    title: 'Custom Title'
  },

  render: function() { /* ... */ }
});

var App = React.createClass({
  componentDidUpdate: function() {
    document.title = this.props.activeRoute.title || 'Default';
  },

  render: function() {
    return dom.div(
      { id: "app" },
      dom.div(null, Link({ to: 'page' }, 'page')),
      this.props.activeRoute()
    );
  }
});

React.renderComponent(
  Route({ handler: App },
        Route({ name: "page", handler: Page })),
  document.body
);

I really enjoy how much this router does for me, but I also feel like it's pretty aggressive at hiding components from me. I guess that forces you to work through the router though. This is the only use-case so far where I feel like it's unnecessary. Can we expose the raw component type with another property, like activeComponent or something?

@ryanflorence
Copy link
Member

Shouldn't your deepest route handler control the title, and therefore have access to its own statics? You can also pass down your title to the active handler <this.props.activeRouteHandler title={title} /> which would allow you to concat titles from parents.

@ryanflorence
Copy link
Member

That said, maybe there are some interesting things people could do if we exposed the current active routes

var title = Router.getActiveRouteHandlers().map(function(handler) {
  return handler.constructor.title;
}).join(' : ');

@jlongster
Copy link
Contributor Author

Shouldn't your deepest route handler control the title, and therefore have access to its own statics?

It could, but then you open the possibility for out-of-sync titles. You want to the title to be the default if the current route doesn't define a title, otherwise set the custom title. If the deepest route sets a custom title, and you navigate away from it, the title should be reset to the default title. That means every single route needs to do the title setting, so it's currently in sync.

I could use a mixin, I suppose. But your right in your second comment; it feels like something that would be neat in the top-level component, allowing you to do more complex things like that.

@jlongster
Copy link
Contributor Author

Actually, I figured it out. I just noticed that you automatically create a ref __activeRoute__ which is the component instance. This works well:

  componentDidUpdate: function() {
    var handler = this.refs.__activeRoute__;
    document.title = handler.title ? handler.title() : 'James Long';
  },

It's up to the component's title function if it wants the title to cascade and include its own children.

@mjackson
Copy link
Member

@jlongster Maybe it would be useful for us to include a getActiveRoute method in a route handler mixin? It would be sugar on top of this.refs.__activeRoute__, but at least your code wouldn't have to use __scaryIdentifiers__.

@ryanflorence
Copy link
Member

I prefer this.refs['>:(><) active route monsters (><):<']

@bobeagan
Copy link
Contributor

I could see a getActiveRoute method being useful for our needs.

@jlongster
Copy link
Contributor Author

Maybe it would be useful for us to include a getActiveRoute method in a route handler mixin?

That'd be fine with me! Any reason to not just name the ref activeRoute instead?

Also, shouldn't the name be getActiveRouteHandler?

@jlongster
Copy link
Contributor Author

Reopening as it sounds like there might be an action item here?

@jlongster jlongster reopened this Jul 23, 2014
@ryanflorence
Copy link
Member

yeah, we'd want the handler, not the route descriptor.

@mjackson
Copy link
Member

Sure, I guess we could just have this.refs.activeRouteComponent. I prefer not to call it "handler" because I think of the handler as being the class, not the component instance. If that's too wordy we can probably think of something else.

@ryanflorence
Copy link
Member

activeRouteHandlerInstance

@mjackson
Copy link
Member

activeRouteHandlerTheFactThatWereExposingThisMakesMeFeelLikeTheresSomeBiggerProblemWithTheAPI

@ryanflorence
Copy link
Member

Perfect. I still think every component just tells something else to deal with the title

@ryanflorence
Copy link
Member

ooh, I just realized you can do this:

<this.props.activeRouteHandler onTitle={this.handleTitle}/>

Child routes can now send a title up to the parent, allowing you to send it up and down as far as you like. Its just data from a child like anything else, right?

@iamrandys
Copy link

I've run into the same issue with trying to set the page title from the activeNestedRouteComponent. This is the only case that I would want to do this that I can think of. Is there any chance we can add a title to the Route config. . That way I don't have to add ANY code to my application and the title is updated for me.

On Jul 23, 2014, at 7:43 PM, Ryan Florence [email protected] wrote:

ooh, I just realized you can do this:

<this.props.activeRouteHandler onTitle={this.handleTitle}/>
Child routes can now send a title up to the parent, allowing you to send it up and down as far as you like. Its just data from a child like anything else, right?


Reply to this email directly or view it on GitHub.

@ryanflorence
Copy link
Member

Isn't title dependent on data in the handler?

<Route path="/topics/:id" handler={DiscussionTopic} title="hmm ..."/>

You'd probably want to set the document title to the discussion topic title when your discussion topic data lands.

Additionally, we really want to make <Route> more extensible (#65) so you could add any title convention you want to the route config, but I don't think that's the job of the router.

@mjackson
Copy link
Member

Agree about #65. We probably wouldn't be having this conversation if we had a RouteHandler mixin.

@ryanflorence
Copy link
Member

yeah, after we settle #112, Route will be about 10 lines of code, making it really easy to do.

@jlongster
Copy link
Contributor Author

The title is dependent on data, yes. Agreed that extensibility > lots of built-in declarations.

ooh, I just realized you can do this:

<this.props.activeRouteHandler onTitle={this.handleTitle}/>

Child routes can now send a title up to the parent, allowing you to send it up and down as far as you like. Its just data from a child like anything else, right?

That would work, but again introduces the problem of a "default" title. All routes would have to call it, even if they didn't want to set a title, to make sure it gets reset to the default. Also, I'm not sure when you would call it? Changing the title isn't really an event, it's just something that needs to stay in sync with the current route.

@sophiebits
Copy link
Contributor

I agree that there should probably be some way to set document.title declaratively.

@ryanflorence
Copy link
Member

I think we just need more extensible primitives and let somebody make a mixin that does it.

@ryanflorence
Copy link
Member

Regarding Titles

Titles are a lot like rendering, I don't think we can do anything generic that works for everybody, seems to me you make a TitleStore, components that care to change the title tell the store what they'd like to do, and the store decides and updates the title.

Regarding Access to activeRouteHandler statics

Since you can now pass down props and you can pass your static data back up through a function, I think we can close this ticket.

@jlongster please reopen if you're like "bummer"

@jlongster
Copy link
Contributor Author

Since you can now pass down props and you can pass your static data back up through a function, I think we can close this ticket.

There's a big problem with that, as I described in #107 (comment)

Can't we just make a method to get the handle ref or just simply name is something better than __activeRoute__? I am kinda like "bummer"...

@ryanflorence ryanflorence reopened this Jul 30, 2014
@mjackson
Copy link
Member

Can't you mix something like this into route handlers that you want to set the title?

var TitleSetter = {
  propTypes: {
    documentTitle: React.PropTypes.string
  },
  getDefaultProps: function () {
    return {
      documentTitle: 'THE DEFAULT TITLE'
    };
  },
  componentDidMount: function () {
    document.title = this.props.documentTitle;
  }
};

It's not perfect, but you get the idea. document.title derives its value from a route handler prop. Child routes that need to build on the title of their parent route can "inherit" the value of their parent's prop using this.props.activeRouteHandler({documentTitleBase: this.props.documentTitle}) or something. You might also need a componentWillUnmount hook that sets the title back to the default when route handlers unmount.

@jlongster
Copy link
Contributor Author

@mjackson I suppose so, that's probably the best alternative solution. It feels better to handle it in the top-level app component, and leverage our nested routing behavior though. If you don't want to add a better way to get the active handler, that's fine, but it seems like there would be other uses for it too.

@ryanflorence
Copy link
Member

var TitleSetter = {
  propTypes: {
    documentTitle: React.PropTypes.string
  },
  getDefaultProps: function () {
    return {
      documentTitle: 'THE DEFAULT TITLE'
    };
  },
  componentDidMount: function () {
    // via <this.props.activeRouteHandler sendTitle={this.handleTitle} /> all
    // the way down the hierarchy so you can send it all the way back up and
    // handle in your top-level app component
    this.props.sendTitle(this.props.documentTitle);

    // though I'd do:
    titleStore.push(this.props.documentTitle);
    // and titleStore.pop(this.props.documentTitle) in `willUnmount`
    // and let the title store deal with it, you get the hierarchy by
    // the position in the array
  }
};

@mjackson
Copy link
Member

@jlongster I'm just hesitant to couple together parents with child routes so strongly. React itself has a secret (public) context property for components that I belive they're planning on getting rid of soon to discourage coupling components together.

@ryanflorence
Copy link
Member

You now know the route's name, which helps a bit. I still think you make a title module and route handlers tell it they have mounted and it decides what to do about the title.

<3 and ;_; @jlongster

@karlmikko
Copy link

Using something like https://github.com/karlmikko/react-router/blob/master/modules/stores/TitleStore.js

If the title is set on every render all the way down, the last rendered item will what is set on the nextTick.

The only issue with this approach I can think of is if the render forks and a shallow path re-renders and changes the title.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 25, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants