Improve usability of clicking on a file in the repository browser
Description
In the file tree Vue refactor, when a user clicks on an empty space in a row, it's the same as if they clicked the file name. Right now we handle this in a click callback, but what if we used native HTML to do this. This way we'd gain the benefits of:
- User can see the href before the click.
- Right clicking causes "Open in New Tab" to appear.
- Meta clicking natively works.
UX
Point 1 - Should we disable click-on-row and only support click on the text of each "clickable" element?This includes the link to modules as seen in this screenshot:
More details from #35250 (closed):
Previous discussion on the topic
-
@pslaughter started a discussion: (+5 comments) question (non-blocking): @mvanremmerden is clicking on the file row a feature we're interested in supporting? Concerns:
- This doesn't add any extra capability which isn't already accomplished by just using the name link.
- For a complete native feel (i.e. features like ctrl+click), there's some extra complexity (i.e. meta vs ctrl in different OS's).
- It looks like this kind of feature has broken user's expectations in the past #24062 (closed).
Not that we have to replicate them, but it looks like GitHub doesn't implement this:
@iamphill sorry to tap the brakes on this small MR
😅
Point 2 - Fully usable links
Make sure the file list supports ctrl+click, right click and open in new tab, etc. — all the native features of a link.
If we still want to keep the click-on-row, one of the possible solutions is:
Style an underlying anchor tag to capture the "negative" space
This option is described in the context below
Context
The following discussion from !19228 (merged) should be addressed:
-
@pslaughter started a discussion: (+3 comments) suggestion: I'm a little concerned that we're re-inventing the wheel (i.e. the anchor element).
- This coupling to the
tagName
is a little concerning. In the future, is it possible that we have aclick
handler on a non-anchor element? - There's some native anchor features that we're still missing (i.e. "Open in New Tab" in context menu, mobile support, seeing the href in the browser).
With a little CSS magic, I think we can actually remove the
openRow
method in favor of an underlying<a>
. What do you think of this? Is it too hacky?diff --git a/app/assets/javascripts/repository/components/table/row.vue b/app/assets/javascripts/repository/components/table/row.vue index 81ae5143082..7305d5de558 100644 --- a/app/assets/javascripts/repository/components/table/row.vue +++ b/app/assets/javascripts/repository/components/table/row.vue @@ -96,23 +96,18 @@ export default { return this.id.slice(0, 8); }, }, - methods: { - openRow(e) { - if (e.target.tagName === 'A') return; - - if (this.isFolder && !e.metaKey) { - this.$router.push(this.routerLinkTo); - } else { - visitUrl(this.url, e.metaKey); - } - }, - }, }; </script> <template> - <tr :class="`file_${id}`" class="tree-item" @click="openRow"> + <tr :class="`file_${id}`" class="tree-item link-overlay-target"> <td class="tree-item-file-name"> + <component + :is="linkComponent" + :to="routerLinkTo" + :href="url" + class="link-overlay" + ></component> <i :aria-label="type" role="img" :class="iconName" class="fa fa-fw"></i> <component :is="linkComponent" :to="routerLinkTo" :href="url" class="str-truncated"> {{ fullPath }} diff --git a/app/assets/stylesheets/framework/common.scss b/app/assets/stylesheets/framework/common.scss index 4b89a2f2b04..2274a3ff3cc 100644 --- a/app/assets/stylesheets/framework/common.scss +++ b/app/assets/stylesheets/framework/common.scss @@ -563,3 +563,22 @@ img.emoji { .gl-font-size-small { font-size: $gl-font-size-small; } .gl-line-height-24 { line-height: $gl-line-height-24; } + +.link-overlay-target { + position: relative; + + a { + position: relative; + z-index: 1; + + &.link-overlay { + position: absolute; + z-index: 0; + display: block; + left: 0; + top: 0; + width: 100%; + height: 100%; + } + } +}
I'm not sure of the cross browser considerations of this. IMO, it's worth considering because it creates a very natural UX
🤔 - This coupling to the
Proposal
Limit the size of the file/folder clickable area while keeping it easy to access the most important element of the row:
- Hovering the row keeps the regular pointer instead of the pointing hand cursor:
- The file/folder hit area is constrained to the “name” table cell. When hovering the file/folder it's underlined.
- Since modules are likely to be the minority, we reduce the hit area to just the commit and module name. The module name link includes the icon.
Availability & Testing
No risks to availability are anticipated.
Cross-browser testing will be necessary to ensure that clicking behaves as expected in all supported browsers.
Unit/integration test coverage should be sufficient. However, there are several end-to-end tests that were written assuming the existing behavior, so package-and-qa
should be run before merging to confirm that the tests are compatible with any changes.