Add repo file tree item link behavior

[bytedream]

Converts the repo file tree items into <a> elements to have default link behavior. Dynamic content load is still done when no special key is pressed while clicking on an item.

[wxiaoguang]

It should also follow the browser's default behavior: Ctrl+Click / Command+Click means "Open in New Window"

[bytedream]

Done, thanks for pointing this out

[wxiaoguang]

By doing some searches, I think "middle key" is not a widely used approach to "open a new window". I do not see browsers officially support it.

The widely supported approach is "Ctrl+LeftClick"

[wxiaoguang]

If you'd like to support "middle click", the ideal approach can be like this:

• make the items to be real <a> links, then middle-click works for users by default.
• use our @click handler to handle ctrl/meta keys, and stop the default link's behavior.

[silverwind]

I think we should just render a native <a> element and only preventDefault the click event with button 0, while letting the browser handle all other interactions.

This way, stuff like middle click, ctrl+click, cmd+click will all work natively and don't even need explicit handlers.

[wxiaoguang]

I think we should just render a native <a> element and only preventDefault the click event with button 0, while letting the browser handle all other interactions.

This way, stuff like middle click, ctrl+click, cmd+click will all work natively and don't even need explicit handlers.

The same as #34730 (comment) ? Or is there something I missed?

[silverwind]

I think we should just render a native <a> element and only preventDefault the click event with button 0, while letting the browser handle all other interactions.
This way, stuff like middle click, ctrl+click, cmd+click will all work natively and don't even need explicit handlers.

The same as #34730 (comment) ? Or is there something I missed?

Basically the same, but I guess the code still need to check whether any modifier key is held, and only e.preventDefault when none are because deciding only on e.button is not enough.

[wxiaoguang]

I see, maybe ideally it could be like this:

• use <a> for tree items with real links.
• if no ctrl/meta/alt key, then use our "fetch" to load the file view content without reload, and preventDefault.
• otherwise, we do nothing and leave the work to the browser.

[silverwind]

Yes, so basically:

if (e.button !== 0 || e.ctrlKey || e.metaKey || e.altKey || e.shiftKey) {
  return; // let browser handle the click
}

e.preventDefault();
// update view to clicked file

Maybe shiftKey can be removed, I'm not aware of any special functionality tied to it. altKey is useful for text selection within a link.

[bytedream]

Thanks for the suggestions, I've updated the PR accordingly by using <a> tags.

By doing some searches, I think "middle key" is not a widely used approach to "open a new window". I do not see browsers officially support it.

Interesting, I almost exclusively use the middle click to open links and such in a new tab.

Maybe shiftKey can be removed, I'm not aware of any special functionality tied to it. altKey is useful for text selection within a link.

Shift+LeftClick opens the link in a new browser window, so I've kept it.

[silverwind]

Yes, we need to keep all modifiers, they all have a purpose and mine opens new window on shift+click too. middle click and ctrl+click are both known to me, so they are not so obscure :)

[silverwind]

You can add one of our link classes from here instead:

gitea/web_src/css/base.css

Lines 253 to 256 in a2ae7c6

	/* a = always colored, underlined on hover */
	/* a.muted = colored on hover, underlined on hover */
	/* a.suppressed = never colored, underlined on hover */
	/* a.silenced = never colored, never underlined */

I guess silenced may be the best fit if you never want to highlight.

[silverwind]

Would extract this into a function in web_src/js/utils/dom.ts:

/** Returns whether a click event is a left-click without any modifiers held */
export function isPlainClick(e: MouseEvent) {
  return e.button === 0 && !e.ctrlKey && !e.metaKey && !e.altKey && !e.shiftKey;
}

[silverwind]

Suggested change

	const doLoadChildren = async (e?: MouseEvent) => {
	const doLoadChildren = async (e: MouseEvent | null) => {

I guess I would prefer if you pass explicit null here to indicate the absence of a event.