Skip to content
This repository was archived by the owner on Dec 4, 2017. It is now read-only.

ToH should use anchors + routerLink instead of router.navigate almost everywhere #998

Closed
IgorMinar opened this issue Mar 22, 2016 · 12 comments
Assignees

Comments

@IgorMinar
Copy link
Contributor

For example in https://angular.io/docs/ts/latest/tutorial/toh-pt5.html

The example code right now looks like this:

<div *ngFor="#hero of heroes" (click)="gotoDetail(hero)" class="col-1-4">
gotoDetail(hero: Hero) {
  let link = ['HeroDetail', { id: hero.id }];
  this._router.navigate(link);
}

This code is problematic because of the following reasons:

  • the navigation via the div is not accessible (to screenreaders and users using keyboard for navigation)
  • it's not possible to left-click+cmd to open the link in a new tab/window
  • it's not possible to right-click on an item and copy the link
  • SEO also suffers because crawlers don't see a link so they can't index the transition to the new state

All these problems can be avoided (and the code can be made shorter) if routerLink directive is used instead:

<a *ngFor="#hero of heroes" [routerLink]="['HeroDetail', { id: hero.id }]" class="col-1-4">

This code will generate a link that has all the good properties listed above, while fully preserving the functionality of the app.

@wardbell
Copy link
Contributor

That is great information and I'd like to capture that in the router chapter as well!

The accessibility arguments for <a> are winners (and I wonder what the material design people are doing about this).

Delegating link parameters array definition

We should note that, if the developer want's component control over the navigation, that's easy to do while still using RouterLink to display the target url.

Template:

  <a *ngFor="#hero of heroes" [routerLink]="gotoDetail(hero)" class="col-1-4">

Component:

  gotoDetail(hero: Hero) {
    let link = ['HeroDetail', { id: hero.id }];
    return link;
    //this._router.navigate(link);
  }

RouterLink or (click) binding?

I need to discuss with John why we used (click) to delegate to a component navigation method rather than using RouterLink. I suspect we did so without thinking it through.

It could be intentional under some circumstances: maybe you do NOT want the user to easily open it in a different tab; I've seen resistance to that AND to deep linking among some of our IdeaBlade clients.

If that's what you want, fine. But you should KNOW that you're making that choice deliberately ... and we'd say so in the docs.

Plunker sadness

I'm somewhat challenged by my present inability to demonstrate the "open in another tab" feature from within plunker. Works find when the app is run independently. Within plunker, we seem to be generating a URL with a base path that is different from the base path of the source tab.

In this snapshot, the top url is the source tab and the bottom url is the generated url for "Narco"

image

Therefore opening the link in a new tab results in a 404. Works fine if I click-in-place. Maybe someone has an idea?

@IgorMinar
Copy link
Contributor Author

Can you share the link to the plunker that is broken in this way?

On Tue, Mar 29, 2016 at 12:04 PM Ward Bell [email protected] wrote:

That is great information and I'd like to capture that in the router
chapter as well!

The accessibility arguments for are winners (and I wonder what the
material design people are doing about this).
RouterLink or (click) binding?

I need to discuss with John why we used (click) to delegate to a
component navigation method rather than using RouterLink. That could be
intentional under some circumstances: maybe you do NOT want the user to
easily open it in a different tab; I've seen resistance to that AND to deep
linking among some of our IdeaBlade clients.

If that's what you want, fine. But you should KNOW that you're making that
choice deliberately ... and we'd say so in the docs.
Plunker sadness

I'm somewhat challenged by my present inability to demonstrate the "open
in another tab" feature from within plunker. Works find when the app is run
independently. Within plunker, we seem to be generating a URL with a
base path that is different from the base path of the source tab.

In this snapshot, the top url is the source tab and the bottom url is the
generated url for "Narco"

[image: image]
https://cloud.githubusercontent.com/assets/129061/14119807/cb745048-f5a4-11e5-8e60-32dfb34e3541.png

Therefore opening the link in a new tab results in a 404. Works fine if I
click-in-place. Maybe someone has an idea?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#998 (comment)

@IgorMinar
Copy link
Contributor Author

I would like to understand the "resistance" to using links more. Sounds
like security via obscurity to me (since once you click the button the link
will be exposed anyway).

Do you know what's the real intention here?

As for material, in v1 they allow which internally
uses an anchor. in v2 they will also allow style
of creating buttons. So while the capability is there, we need to do a
better job of educating developers about how to use buttons correctly when
doing deep navigation.

On Tue, Mar 29, 2016 at 12:13 PM Igor Minar [email protected] wrote:

Can you share the link to the plunker that is broken in this way?

On Tue, Mar 29, 2016 at 12:04 PM Ward Bell [email protected]
wrote:

That is great information and I'd like to capture that in the router
chapter as well!

The accessibility arguments for are winners (and I wonder what the
material design people are doing about this).
RouterLink or (click) binding?

I need to discuss with John why we used (click) to delegate to a
component navigation method rather than using RouterLink. That could be
intentional under some circumstances: maybe you do NOT want the user
to easily open it in a different tab; I've seen resistance to that AND to
deep linking among some of our IdeaBlade clients.

If that's what you want, fine. But you should KNOW that you're making
that choice deliberately ... and we'd say so in the docs.
Plunker sadness

I'm somewhat challenged by my present inability to demonstrate the "open
in another tab" feature from within plunker. Works find when the app is run
independently. Within plunker, we seem to be generating a URL with a
base path that is different from the base path of the source tab.

In this snapshot, the top url is the source tab and the bottom url is the
generated url for "Narco"

[image: image]
https://cloud.githubusercontent.com/assets/129061/14119807/cb745048-f5a4-11e5-8e60-32dfb34e3541.png

Therefore opening the link in a new tab results in a 404. Works fine if I
click-in-place. Maybe someone has an idea?


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#998 (comment)

@wardbell
Copy link
Contributor

It is hard for me to explain the resistance because I don't always find the reasons all that compelling.

For example, many say that deep-linking into your bank account isn't something they want to encourage. Maybe this is faux security.

I've heard a UX reason that aligns with resistance to the old MDI paradigm: users get lost when there are a bunch of open windows into the app ... some of which they forget to close.

These kinds of reasons motivated one client to avoid the router altogether in their A1 app. Navigation was controlled entirely through traditional menus and buttons. It worked fine for them (and this is one reason I'm hoping we don't couple async module loading TOO closely to the router, even thought that is a natural fit).

Here is that plunker: http://plnkr.co/edit/NiMRE6EYUEwElODjyDiW?p=preview

@wardbell
Copy link
Contributor

@IgorMinar: had this been a button would we feel the same way? Should buttons expose urls that can be opened in another tab?

Before saying "of course", I observe that _very few of the buttons on this github page do that. Is github "wrong"?_

Given that the Dashboard renders those links as buttons, I'm waffling on how I come down on the use of RouterLink in ToH! If github doesn't support "open in another tab", why must ToH do so?

I DO get why the <div> should be replaced by something ... either <a> or <button>

I've invited Marcy and Almero to chime in.

@IgorMinar
Copy link
Contributor Author

I think my point is that buttons should never just tell the router to
navigate somewhere on click as the only thing they do. If that's the only
thing they do then you should use anchors instead.

If however button executes an action (e.g. save a form) and then they
navigate somewhere, you should not use anchors.

The way I think about this is when I would use GET vs POST in the old days
when building a form that does a full page reload. GET = [routeLink] and
POST = (click). And if you don't remember, GET should be used for all
idempotent actions and POST/DELETE/PUT/PATCH should be used for all
non-idempotent actions.

On Tue, Mar 29, 2016 at 12:48 PM Ward Bell [email protected] wrote:

It is hard for me to explain the resistance because I don't always find
the reasons all that compelling.

For example, many say that deep-linking into your bank account isn't
something they want to encourage. Maybe this is faux security.

I've heard a UX reason that aligns with resistance to the old MDI paradigm
https://en.wikipedia.org/wiki/Multiple_document_interface: users get
lost when there are a bunch of open windows into the app ... some of which
they forget to close.

These kinds of reasons motivated one client to avoid the router altogether
in their A1 app. Navigation was controlled entirely through traditional
menus and buttons. It worked fine for them (and this is one reason I'm
hoping we don't couple async module loading TOO closely to the router, even
thought that is a natural fit).

Here is that plunker: http://plnkr.co/edit/NiMRE6EYUEwElODjyDiW?p=preview


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#998 (comment)

@wardbell
Copy link
Contributor

That's a good intuition, Igor. Yet it feels a wee bit incomplete.

The ToH editor has a back button. All it does is navigate back to the screen you were on. Personally, I wouldn't want that button to display a url nor would I think it intuitive to "go back" by opening in a new browser tab.

I do not have the answers. I have instincts derived from my experience of using apps ... just like all of us.

I'll bet that people who do app UX for a living have some rules of thumb that we can sample. Before we set this "in stone", I think we should gather some of those rules.

Do you have a UX expert/book that you refer to? Anyone is welcome to chime in.

On an interim basis, I'd invite developers to ask themselves

  • does it make sense for the user to click here and open in a new tab?
  • would it be useful to be able to email this link to someone?
  • ...

@gkalpak
Copy link
Member

gkalpak commented Mar 29, 2016

I'm pretty far from being a UX-expert, but I can't think of any benefit of making it difficult to open something in a new tab, espacially for an app with mainly static content, such as the docs.

And even in other kinds of applications (more "application-y"), I always get frustrated when "open in new tab" doesn't work. It's not that anyone will force a user to open something in a new tab, but if the user decides to do so, we shouldn't stand in their way because:

a. It's a very typical behavior that users are used to and expect.
b. We can't really prevent them from opening something in a new tab anyway (e.g. copying the URL into a manually opened tab).

@IgorMinar
Copy link
Contributor Author

I think it's better to error on the side of overusing anchors than
underusing them. Links/anchors is what makes the web work. Having a few
extra links is not a big deal from UX perspective, not having them does
impact UX in a big way.

I just read Georgios' comment and I think we think alike. It's very rare
that using a link actually breaks something, the opposite is no the case.

On Tue, Mar 29, 2016 at 1:30 PM Ward Bell [email protected] wrote:

That's a good intuition, Igor. But how about this.

The ToH editor has a back button. All it does is navigate back to the
screen you were on. Personally, I wouldn't want that button to display a
url nor would I consider "going back" by opening in a new browser tab.

I do not have the answers. I have instincts derived from my experience
of using apps ... just like all of us.

I'll bet that people who do app UX for a living have some rules of thumb
that we can sample. Before we set this "in stone", I think we should gather
some of those rules.

Do you have a UX expert/book that you refer to? Anyone is welcome to chime
in.


You are receiving this because you were mentioned.

Reply to this email directly or view it on GitHub
#998 (comment)

@AlmeroSteyn
Copy link
Contributor

Hey guys :-) My five cents.

Firstly I think it is a good idea to step away from the clickable div here for all the reasons Igor stated in the opening post. With HTML semantics it is generally a good idea to use the already defined elements for a specific job rather than making another element pretend to be something else, unless there is a really good reason. And if we use the div here it is missing ARIA anyway.

Which leaves us with button vs. link.

I also think it should be a link.

Look at the ARIA definitions of the roles 'button' and 'link' in the W3C spec:
Link role
Button role

Links should be used for hyperlinks to internal or external resources. Which is exactly what we are doing with the routes.

Buttons are also expected to be triggered with Spacebar and links with Enter, so this choice can have an impact on the expected keyboard navigation experience. Especially if we are mixing elements on one page. Links are also recognised by assistive technologies as part of the navigation of the site.

And with resources becoming more readable, the user is tecnically able to navigate the application through the browser address bar. By hiding the link destination the user only has to click it once and can then always go there via bookmark or typing it. So the button option will not impose 'security' but rather hamper users who want to navigate more freely.

IgorMinar added a commit to juleskremer/houston that referenced this issue Mar 30, 2016
@filipesilva
Copy link
Contributor

Regarding the deep linking scenario, I think the problem is actually one of application state.

The issue with an app that misbehaves when deep linked is that it's state was not correctly initialized on boot. If whatever route should not be accessed to anonymous users, the app should kick the user back to an authentication page.

On Angular 1, using ui-routers $urlRouterProvider.deferIntercept() helped massively with this scenario.

@wardbell
Copy link
Contributor

wardbell commented Nov 1, 2016

Adopting this [routerLink] binding suggestion for ToH 5 in PR #2718

Kept the button-click imperative navigation in HeroesComponent mainly to show how to do that when you need imperative navigation but also because this use case is more button-like than anchor-like.

@wardbell wardbell closed this as completed Nov 1, 2016
wardbell added a commit to IdeaBlade/angular.io that referenced this issue Nov 3, 2016
wardbell added a commit to IdeaBlade/angular.io that referenced this issue Nov 3, 2016
wardbell added a commit that referenced this issue Nov 3, 2016
* docs(toh-5): dashboard uses [routerLink] bindings #998
closes #998

* chore: temp add toh-5 to bad-code-excerpt-skip-patterns.txt
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

5 participants