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

client: Middle click on title to open entry in new tab #1471

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PhrozenByte
Copy link
Contributor

@PhrozenByte PhrozenByte commented Nov 23, 2023

Ten years after #452 (I'm feeling old...) it's time to try again to finally bring this tech-wise tiny but UX-wise major feature to upstream. I'm not aware of any good reason why one should be required to click on the tiny favicon to open an entry in a new tab.

I believe that this should be default behaviour. There's still the possibility to make this an opt-in feature using a new config.ini option, but I'd strongly discourage it - it's simple and basic, it doesn't affect other users, thus it shouldn't come with unnecessary complexity (like an option).

If you want to merge both #1470 and #1471, ignore this PR and merge #1472 instead. Merging any of the three PRs will automatically close the other two PRs.

Closes #1470
Closes #1472

Copy link

netlify bot commented Nov 23, 2023

Deploy Preview for selfoss canceled.

Name Link
🔨 Latest commit 845c857
🔍 Latest deploy log https://app.netlify.com/sites/selfoss/deploys/655fe4a778a42f00088b21ea

@PhrozenByte
Copy link
Contributor Author

Related issue: #1411

@jtojnar
Copy link
Member

jtojnar commented Nov 26, 2023

I think this makes sense. Using a link might be a bit iffy from accessibility standpoint but then again, we used ARIA role=link before so this should not be a regression.

className="entry-title-link"
aria-expanded={expanded}
aria-current={selected}
role="link"
tabIndex="0"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer necessary either.

@jtojnar
Copy link
Member

jtojnar commented Dec 3, 2023

Actually, now I remember why we do not do this. Some spouts (e.g. Twitter) contain links in the title and links cannot be nested. I guess we can fall back to the old way when the title contains < character.

@jtojnar jtojnar added this to the 2.20 milestone Dec 3, 2023
@jtojnar jtojnar removed this from the 2.20 milestone Apr 14, 2024
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

Successfully merging this pull request may close these issues.

2 participants