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

feat(editor): add "Open in new tab" option for link previews #6674

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

Conversation

pbirrer
Copy link

@pbirrer pbirrer commented Nov 21, 2024

This update aims to improve accessibility to files added as previews in the text editor.

📝 Summary

  • Enhanced PreviewOptions.vue to include a new "Open in new tab" button for link previews.

    • Introduced the openLink method to open the link in a new browser tab.
    • Added OpenIcon for the button to improve UI consistency.
    • New href prop added to handle link URLs dynamically.
  • Updated extractLinkParagraphs.js to extract href attributes from nodes.

    • Ensures link previews include the necessary href for the "Open in new tab" action.

🖼️ Screenshots

text-open-option-link-preview

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits
  • Tests (unit, integration and/or end-to-end) passing and the changes are covered with tests
  • Documentation (README or documentation) has been updated or is not required

@max-nextcloud
Copy link
Collaborator

Dear @pbirrer

Thanks a lot for your contribution. I really like the idea and have been missing this in the past.
I wonder if it would make sense to get this out of the three dot menu as a separate button left of it. That would make it more visible and also distinguish it from the preview settings in the dropdown.

However this could also be a follow up.

I will add this PR to my review list and do a code review as soon as possible.

@max-nextcloud max-nextcloud self-assigned this Dec 3, 2024
@rvjr
Copy link

rvjr commented Dec 3, 2024

From a usability point of view I would also prefer a direct button; if this is okay with the NC team. It would make the function much more obvious.

Copy link
Contributor

github-actions bot commented Dec 6, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

(If you believe you should not receive this message, you can add yourself to the blocklist.)

Copy link
Collaborator

@max-nextcloud max-nextcloud left a comment

Choose a reason for hiding this comment

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

Code looks good. There are a few test failures because of the additional href property in extractLinkParagraph. They are fairly easy to spot in the Files changed view here on github.

@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Dec 9, 2024

@pbirrer could you try and fix the tests?
You should be able to run them locally with

npx vitest src/tests/plugins/extractLinkParagraphs.spec.js

Please let me know if you need any help with this.

@max-nextcloud

This comment was marked as resolved.

@juliusknorr
Copy link
Member

I would say a button is still great to have in addition to the clickable link preview. Especially since the link preview might be interactive or rendered by external apps, so I'd always prefer to still have a common way of just opening the link

@pbirrer pbirrer force-pushed the feature/add-open-link-preview-option branch 2 times, most recently from c12a909 to 8a87b02 Compare December 18, 2024 10:18
@mejo-
Copy link
Member

mejo- commented Dec 18, 2024

Thanks, I also agree that a button to open the link is still a good idea. But I wonder whether the button should be in the link bubble instead of the three-dot-menu. I perceive the three dot menu more like a "config menu", also something normal users probably won't spot that easily. Most people will just click on the link and see the link bubble that opens. I would expect a "Open in new tab" button there.

@pbirrer
Copy link
Author

pbirrer commented Dec 18, 2024

@mejo-: From what I can tell, the link bubble becomes inaccessible in preview mode. To resolve this, the button could be placed nearby—perhaps beside or below the triple-dot button?

@juliusknorr
Copy link
Member

juliusknorr commented Dec 18, 2024

From what I can tell, the link bubble becomes inaccessible in preview mode. To resolve this, the button could be placed nearby—perhaps beside or below the triple-dot button?

Preview/read only documents should also show the preview bubble, so I'd agree to always place it in there. It also will make it available for inline links that are not a standalone paragraph, as then we do not show the menu.

Screenshot 2024-12-18 at 23 25 07

Edit: Though I would also be ok with keeping the action in the three dots as well. Might be more accessible for users if the menu is shown anyways

Copy link

codecov bot commented Dec 18, 2024

Codecov Report

Attention: Patch coverage is 69.23077% with 4 lines in your changes missing coverage. Please review.

Project coverage is 46.79%. Comparing base (c76f190) to head (2c673b6).

Files with missing lines Patch % Lines
src/components/Editor/PreviewOptions.vue 63.63% 4 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #6674      +/-   ##
==========================================
+ Coverage   46.76%   46.79%   +0.03%     
==========================================
  Files         748      733      -15     
  Lines       34163    34156       -7     
  Branches     1242     1227      -15     
==========================================
+ Hits        15976    15985       +9     
+ Misses      17565    17564       -1     
+ Partials      622      607      -15     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@max-nextcloud
Copy link
Collaborator

@juliusknorr

Edit: Though I would also be ok with keeping the action in the three dots as well. Might be more accessible for users if the menu is shown anyways

Just to clarify - that would mean the same action in two places, right?

@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Dec 19, 2024

Ahh... I think now i get the three dot menu - you're looking into things that have an embedded preview like whiteboards and so on, right?

grafik
(Using a different text document with a mermaid diagram here for illustration as currently whiteboards have 0 hight)

There's no link here right now that takes one to the actual whiteboard.
In this case I think it would be better to put the link next to the " ✏️ Edit" - button.

@max-nextcloud
Copy link
Collaborator

All that said... I will fix the linter and prepare this for merging as it's already an improvement over the current state. We can move the link to more meaningful places (i.e. into the link bubble and the preview) as a follow up.

@max-nextcloud max-nextcloud force-pushed the feature/add-open-link-preview-option branch from 8a87b02 to f0e350f Compare December 19, 2024 09:50
- Enhanced `PreviewOptions.vue` to include a new "Open in new tab" button for link previews.
  - Introduced the `openLink` method to open the link in a new browser tab.
  - Added `OpenIcon` for the button to improve UI consistency.
  - New `href` prop added to handle link URLs dynamically.

- Updated `extractLinkParagraphs.js` to extract `href` attributes from nodes.
  - Ensures link previews include the necessary `href` for the "Open in new tab" action.

These changes improve the user experience by allowing quick access to link previews in the editor, streamlining navigation workflows.

Signed-off-by: Peter Birrer <[email protected]>
Signed-off-by: Max <[email protected]>
@max-nextcloud max-nextcloud force-pushed the feature/add-open-link-preview-option branch from f0e350f to 2c673b6 Compare December 19, 2024 13:50
@mejo-
Copy link
Member

mejo- commented Dec 19, 2024

Thanks to everyone who looks into this. I just want to stress the fact that non-techy users expect a link to be accessible through clicking the link. So I still think that the link bubble that opens when clicking the link (the text one, not the preview) should have a "open in new tab" button in my opinion. That's also been requested here: nextcloud/collectives#1226

For link previews (where the link bubble doesn't show) I would prefer to have the button as an overly button next to the "edit" button, like @max-nextcloud suggested.

When I watch people using Text/Collectives, my impression is that the three-dots menu left to links often remains unnoticed. Plus, as @juliusknorr already pointed out, it's only displayed for links that span over a separate paragraph, not for inline links.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

5 participants