Ajax links should still be real links #953

Open
JorisDebonnet opened this Issue Feb 20, 2014 · 7 comments

Comments

Projects
None yet
4 participants

Everything loads with ajax now. That's smooth and everything, but it causes some annoyances here and there.

One such thing is when I want to open multiple links in multiple tabs: that is currently not possible. For example if I have a list of 7 Files, and I want to open all of them in separate tabs in order to then make the same change to all of their Names.

If I middle click though, that's interpreted as a click and it just loads in the same window. Even when I right click to inspect what the link looks like - the right click loads the new page.

  1. Middle click and right click should never open a new page in the current window.
  2. It would be very useful to have actual links (i.e. <a> tags), and overwrite their default behavior -for left clicks- to load with ajax. After all, all links in overview pages like that send simple GET requests, right? That means it's perfectly possible to generate an <a> there as well.

Apologies if this was already reported, was a bit hard figuring out the right terms for this issue.

@chillu chillu added the enhancement label Feb 22, 2014

@chillu chillu referenced this issue in silverstripe/silverstripe-framework Feb 22, 2014

Closed

Default browser actions on right click in GridField #2890

Owner

chillu commented Feb 22, 2014

Hey Joris, good suggestions, thanks for putting so much effort into describing the motivations for this issue. I've created a simple fix for the right/middle click actions with silverstripe/silverstripe-framework#2890. It allows right/middle clicks on the "edit" icon by defaulting back to the browser's standard link behaviour. Changing all <td> nodes to contain their own <a> will be difficult, since the edit link is architecturally decoupled from these columns (the navigation event is triggered through JS instead). I've tried my best to deal gracefully with middle clicks and CMD+left click on OSX there. Not quite sure how Windows behaves (Browserstack isn't very helpful when it comes to click behaviour), would you be able to try this out on a "native" IE? I'm interested both in middle clicks and keyboard shortcuts to open a new tab (presumably through CTRL+ left click?).

chillu added a commit to chillu/silverstripe-framework that referenced this issue Feb 22, 2014

Default browser actions on right click in GridField
Allows opening into a new tab from "edit" buttons,
and in the case of Firefox allows right clicks on the row
without causing an action (e.g. to inspect the element).

Fixes silverstripe/silverstripe-cms#953

I'm always confused whether I should reply in the bug request or in the code proposal - but let's do it here!

That change fixes it mostly for GridFields: right-clicking doesn't cause an action in the cms (but does still show the context dropdown), and middle-clicking opens a window. The problem with the latter is that the popup blocker blocks it. I can turn it off of course, so it can work, but it's of course not ideal.
It does not work on e.g. Folders in the Files structure. Middle- or right clicking works as above when clicking on Files, but Folders still have the old behavior.
Control-click (indeed the way to open a link in a new tab) does not work: it does not open any new window, and it does open the link in the current one. But all that is getting cumbersome to emulate in javascript...
There's also a single error in the current implementation: GridField.js line 251 needs the parameter e in the click function.

I don't know SilverStripe well enough yet to know where everything happens, but is there no way to wrap the contents of those <td> nodes with appropriate <a> links, as sooon as the overview loads? Then the left-click on those <a> elements could be preventDefault'ed (but still propagated so the td event can do its ajax thing), while other clicks (middle and right) allow for all natively supported things. After all, another thing one might want to do is right click and choose "open in a new window" or "open in a new tab", which is completely impossible without actual <a> tags.

The mouse clicks work exactly as I'd expect them to in the menu (Pages / Files / ...) as well as in the Pages overview, as well as on the tabs inside Root in edit views,.. which is why it would be consistent and nice if it worked, somehow, in other places too :)

I notice that the edit-icon-links at the right of gridfield rows work as completely native links: they don't ajax-open the links. If you were to make more changes in this area, perhaps that could be fixed as well - the point being that all tds (except col-buttons) in the whole row get inner-wrapped with the same <a>s, and then all <a>s in the row (incl the icon link) getting a different behavior for their left-clicks (but native ones for other clicks).

It is then still possible to click inside the td and next to an a, but that feels native enough to me at least - if you want native link functionality, you click on the actual link (the linked text); the area behind it is intuitively ajax-activated.

Contributor

tractorcow commented Feb 24, 2014

I think the idea of wrapping the cell contents with <a> tags might work, but you would end up with CSS positioning issues (making the tag fill the entire cell) plus it would possibly break a some gridfield extension modules.

Although I agree with right-click not navigating to the record, I'm afraid I'm not overly fond of the window.open solution because, as @JorisDebonnet points out, it bypasses things like keyboard modifiers and triggers popup blockers. Is it obvious enough that middle click opens in a new window? (or am I the only one who uses shift + click?). I'd rather have no behaviour than risk unexpected behaviour.

How about a better solution: Leave the current method for navigating to the record in place, and make the buttons the hotspot you can use to open in a new tab, window, bookmark, etc...

Which kind of how it currently works :)

Is this also an appropriate place to voice issues about the buttons being too tiny and lacking rollover states? I usually expect elements that react to clicks (especially <a> tags) to respond to the mouse.

Contributor

tractorcow commented Feb 24, 2014

I suppose you could put the <a> after the content in the cell, and then do.

td {
position: relative;
}
td a.itemlink {
    position: absolute;
    top: 0; left: 0; right: 0; bottom: 0;
}

It feels a little awkward still, but it would probably give you a non-ajax friendly view of that record at least. :)

Unfortunately position:relative does not work (as expected) on tds - you can't absolutely position elements within them. The only way is to put another element inside the td first that does support that ... anyway, that's probably getting too awkward too quickly :)

I've been fiddling a bit with the styles, and the only good way seems to be to wrap the text in <a>s and at the same time putting a class on the td, in order to strip the td of its padding, put it onto the a instead, and make the a display:block. That effectively fills up the entire td with the a, and looks very pretty imo: the text is blue since it's a link, and the :hover happens as soon as you mouse over anywhere in the table cell.

In other words,

/* strip td of its padding */
.cms table.ss-gridfield-table tr td.has_a {padding:0}
/* put padding on a instead */
.cms table.ss-gridfield-table tr td.has_a > a {padding:8px;display:block}

and something like

$(".ss-gridfield-item").each(function(){
  // each row
  var tr = $(this), 
    // the edit link URL
    URL = tr.find("a.edit-link").attr("href"); 
  // wrap the contents of every column (except col-buttons) with an <a>, and apply styling
  tr.find("td:not(.col-buttons)").addClass("has_a").wrapInner('<a href="'+URL+'"></a>'); 
});
Contributor

tractorcow commented Feb 24, 2014

If it doesn't work, I guess I'll take your word for it...

You also have to keep in mind that some cells will have content with different height, so it can be hard otherwise to ensure that the height of all links in each cell is consistent and 100%.

I think we should back up a bit... even if we could make each cell linkable, is this the desired behaviour? What do you think @chillu?

@simonwelsh simonwelsh added the master label Mar 16, 2014

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment