Skip to content
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

_fixInitState using hash mode should set initial hash #19

Open
jorgecasar opened this issue Oct 16, 2015 · 5 comments
Open

_fixInitState using hash mode should set initial hash #19

jorgecasar opened this issue Oct 16, 2015 · 5 comments
Labels
Milestone

Comments

@jorgecasar
Copy link

Using mode === HASH the method _fixInitState do nothing. I think it should set the hash to this.prefix + hashInPathName.

I would like to suggest a refactor like following on _fixInitState: https://github.com/leeluolee/stateman/blob/master/src/histery.js#L179-L193

_fixInitState: function(){
  var hash = this.location.hash.replace(this.prefix, "");
  if( this.mode === HISTORY && hash) {
    history.replaceState({}, document.title, _.cleanPath(this.root + hash));
  } else {
    var hashInPathName = _.cleanPath(this.location.pathname.replace(this.rRoot, ""));
    if(this.html5) {
      this.location.replace(this.root + this.prefix + hashInPathName);
    } else {
      this.location.hash = this.prefix + hashInPathName;
    }
  }
}

I made a work around using history:change event, but It's dirtier:

stateman.on('history:change', function(path){
  if( path === '' ) {
    this.stateman.nav('/', {replace: true});
  }
}.bind(this))
@jorgecasar jorgecasar changed the title Fix Init State using hash mode _fixInitState using hash mode should set initial hash Oct 16, 2015
@leeluolee
Copy link
Owner

In fact, mode isn't a public option for developer . The recommended way (see Reference#start) to pick HASH mode is: setting html5 !== true or run stateman in browser that doesn't support html5 history api . mode is only a private param for test (maybe it should prefixed with _ , :( ) .

_fixInitState have two responsibility , and will work when starting stateman with flag html5.

  1. the browser doesn't support html5 history and user visit site via html5 way, the code is

      if( this.mode !== HISTORY && this.html5) {
       if(hashInPathName)  // fix it
    }

    Then we can fix url from /blog/list to /blog#/list, If user visit /blog/list in stupid & old browser . We think it highly possible that the url was pasted from other browser that support html5 .

  2. As well as previous case, It is very probable that user visit site via __Hash way __ in modern browser that support html5. the code:

if( this.mode === HISTORY ) {
  if(hash) // fix it 
}

Then we can also fix It from /blog#/list to /blog/list . This case is also highly possible because user may use the url pasted from old browser.

Beyond that, _fixInitState do nothing else.

So, your contribution may cause some issues, For example , visit the demo exmaple/layout.html .

But I find that _fixInitState is very obscure to understand Since It only works when html5===true.

@leeluolee
Copy link
Owner

For notfound case. You can code like

stateman.on('notfound', function(){
   this.go('app.homepage'); // or this.nav  as well
})

See API for more detail

history:change just a private event .

@jorgecasar
Copy link
Author

The issue is that I'm using html5=false and first load of the page return not found. I want the page 404, then redirection is not an option. First load of the app should redirect to /#/ o whatever the developer wants to be the homepage instead of return a 404. If I have the state 404 and I redirect to this on notfound event, I have to check all the time if is or not the first time (first time path is empty). I think this should be solved from the router. As I said, first time path is empty, I can check it on notfound but I think it's a extra job we can avoid if the config set a option to say what is the initial state of the application or set the diference between location.pathname and confg.root.

@leeluolee
Copy link
Owner

I think I know what you mean.

You have different logic between 『first load 』 and 『runtime 404』, right?

But,

if using html5=false, root and _fixInitState are all meaningless in this version.

what you want is:

  • if user visit /root/path/to , fix it to /root#/path/to.

But, you are using html5=false. It is almost impossible that user will use this url to visit your application.

what you may need is :

  • if if user visit /path without hash , fix it to '#/'.

I think it is a good idea. but instead of redirecting to #/ automately, we may need to provide a option to developer like

stateman.start({
   defaults: '/user/id'
})

So, what is your opinion ?

@jorgecasar
Copy link
Author

Yes, it's just I meant. I thought that _fixInitState was a good place to do that. Doing like I suggested, the migration to html5 = true would be less painful, in fact would be compatible. If you set root: location.pathname you have defaults === '/'.

What do you think about this solution?

_fixInitState: function(){
    var pathname = _.cleanPath(this.location.pathname), hash, hashInPathName;
    // dont support history popstate
    if( this.mode !== HISTORY){
      hashInPathName = pathname.replace(this.rRoot, "")
      if( this.html5 ){
        // config the html5 mode
        if(hashInPathName) this.location.replace(this.root + this.prefix + hashInPathName);
      } else {
        // fallback to HASH mode.
        if( this.defaults ) {
          hashInPathName = this.defaults;
        }
        this.location.hash = this.prefix + hashInPathName;
      }
    }else if( this.mode === HISTORY /* && pathname === this.root*/){
      hash = this.location.hash.replace(this.prefix, "");
      if(hash) history.replaceState({}, document.title, _.cleanPath(this.root + hash))
    }
}

@leeluolee leeluolee added this to the v0.2.1 milestone Oct 21, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants