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

Ugly named route group '/' urls #130

Closed
vladshcherbin opened this issue May 28, 2015 · 12 comments
Closed

Ugly named route group '/' urls #130

vladshcherbin opened this issue May 28, 2015 · 12 comments

Comments

@vladshcherbin
Copy link
Contributor

Example: user group routes (named)

var usersGroup = FlowRouter.group({
  prefix: '/admin/users'
});

usersGroup.route('/', {
  name: 'admin.users.index',
  action: function () {
    FlowLayout.render('adminBasicLayout', {main: 'adminUsersIndex'});
  }
});

usersGroup.route('/create', {
  name: 'admin.users.create',
  action: function () {
    FlowLayout.render('adminBasicLayout', {main: 'adminUsersCreate'})
  }
});

Now we want a link for a named route

FlowRouter.path('admin.users.index');

The route for the / action will be:

http://localhost:3000/admin/users/

The last / is very ugly, can we remove it?

@arunoda
Copy link
Contributor

arunoda commented May 29, 2015

Why not we can remove it. I will mark this as a bug. So, we can work on
this.
On 2015 මැයි 29, සිකු at පෙ.ව. 12.01 Vlad Shcherbin <
notifications@github.com> wrote:

Example: user group routes (named)

var usersGroup = FlowRouter.group({
prefix: '/admin/users'
});

usersGroup.route('/', {
name: 'admin.users.index',
action: function () {
FlowLayout.render('adminBasicLayout', {main: 'adminUsersIndex'});
}
});

usersGroup.route('/create', {
name: 'admin.users.create',
action: function () {
FlowLayout.render('adminBasicLayout', {main: 'adminUsersCreate'})
}
});

Now we want a link for a named route

  • The route for the / action will be:

    http://localhost:3000/admin/users/

    The last / is very ugly, can we remove it?


    Reply to this email directly or view it on GitHub
    #130.

    @arunoda arunoda added the bug label May 29, 2015
    @delgermurun
    Copy link
    Contributor

    @arunoda we had discussion about it on pull/69.

    So should we accept blank ('') path on route?

    @arunoda
    Copy link
    Contributor

    arunoda commented Jun 4, 2015

    I think this is not about that. We need to keep it as like that.
    What he's referring to the last "/".
    We can remove it when we build the path.

    @vladshcherbin
    Copy link
    Contributor Author

    yes, when building a path, we can remove the last / and it fill be fine

    @delgermurun
    Copy link
    Contributor

    Okay, got it.

    @vladshcherbin
    Copy link
    Contributor Author

    @lmacsen hi, why do you suggest it be optional? The urls look ugly, unfriendly and leave a feeling of something unfinished. I really don't know, who and why would use trailing slashes. The choice is ok, but I don't think we need it.

    @vladshcherbin
    Copy link
    Contributor Author

    @lmacsen we are not changing the url behaviour in the browser, only the generated link path (in <a> for example).

    So, the links /users and /users/ would work the same and show you the users list ;)

    @rclai
    Copy link

    rclai commented Jun 16, 2015

    Trailing slashes seem like they are still a problem.

    I have a route that redirects to another route using the route name and it appends slashes at the end.

    FlowRouter.route('/otherroute/:param1?/:param2?', {
      name: 'otherroute',
      action: function () {
        // Do something...
      }
    });
    
    FlowRouter.route('/', {
      name: 'home',
      action: function () {
        FlowRouter.go('otherroute');
      }
    });

    I think it has to do with those optional params.

    @delgermurun
    Copy link
    Contributor

    @rclai It isn't directly related to this bug. Can you create separated issue?

    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

    No branches or pull requests

    5 participants
    @arunoda @delgermurun @rclai @vladshcherbin and others