Skip to content

Conversation

@mejo-
Copy link
Member

@mejo- mejo- commented May 7, 2025

Fixes: #6843
Fixes: nextcloud/collectives#1740

Fixes a regression from #6609

🏁 Checklist

  • Code is properly formatted (npm run lint / npm run stylelint / composer run cs:check)
  • Sign-off message is added to all commits

@mejo- mejo- requested a review from JuliaKirschenheuter May 7, 2025 15:32
@mejo- mejo- self-assigned this May 7, 2025
@mejo- mejo- requested a review from max-nextcloud as a code owner May 7, 2025 15:32
@mejo- mejo- added bug Something isn't working 2. developing labels May 7, 2025
@github-project-automation github-project-automation bot moved this to 🧭 Planning evaluation (don't pick) in 📝 Office team May 7, 2025
@codecov
Copy link

codecov bot commented May 7, 2025

Codecov Report

Attention: Patch coverage is 9.09091% with 10 lines in your changes missing coverage. Please review.

Project coverage is 52.34%. Comparing base (88462c5) to head (f4bf663).
Report is 7 commits behind head on main.

Files with missing lines Patch % Lines
src/nodes/ImageView.vue 0.00% 10 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7201      +/-   ##
==========================================
- Coverage   52.35%   52.34%   -0.01%     
==========================================
  Files         479      479              
  Lines       41669    41681      +12     
  Branches     1023     1023              
==========================================
+ Hits        21815    21818       +3     
- Misses      19752    19761       +9     
  Partials      102      102              

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

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@mejo-
Copy link
Member Author

mejo- commented May 7, 2025

@max-nextcloud I'm curious what you think about the approach with setMeta().

The challenge is to detect whether the loaded attachment just got inserted or whether it's an existing attachment in the node view ImageView.vue. New attachments are uploaded and inserted into the editor content in MediaHandler.vue.

I had to find a way to register the newly inserted image from MediaHandler.vue and then check in ImageView.vue whether the loaded image is the one just inserted. I chose a transaction metadata property as to my knowledge that's kept in the local instance and not synchronised via the collaboration plugin, but I might be wrong here.

@mejo- mejo- moved this from 🧭 Planning evaluation (don't pick) to 🏗️ In progress in 📝 Office team May 7, 2025
@mejo- mejo- force-pushed the fix/focus_image_description_on_insert branch from f941aab to f4bf663 Compare May 7, 2025 15:55
@mejo- mejo- moved this from 🏗️ In progress to 👀 In review in 📝 Office team May 7, 2025
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.

I think this change does what it's intended to do.
However I think the underlying problem is a different one.

I think the alt text should be focused directly after inserting the image - even if the image has not been loaded yet. But it seems we only render the alt text once the image has been loaded. We should always render the alt text. If the image fails to load or takes a long time the alt text might tell what is supposed to be displayed.

Also - if i insert an image and the processing takes a long time so I continue typing somewhere else I would not expect the focus to be moved once the image finished loading.

Copy link
Contributor

@JuliaKirschenheuter JuliaKirschenheuter left a comment

Choose a reason for hiding this comment

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

works, thank you!

@juliusknorr juliusknorr merged commit fa11d97 into main May 8, 2025
64 of 66 checks passed
@juliusknorr juliusknorr deleted the fix/focus_image_description_on_insert branch May 8, 2025 19:48
@github-project-automation github-project-automation bot moved this from 👀 In review to ☑️ Done in 📝 Office team May 8, 2025
@mejo-
Copy link
Member Author

mejo- commented May 8, 2025

/backport to stable31

@emberfiend
Copy link
Contributor

I think this patch introduced this behaviour (video below). Clicking around causes only the isLastInserted ImageView to reload. Importantly, it only happens when you click in between paragraphs...?! I don't understand enough yet about how these parts interact to suggest a fix though 😅

Screencast.from.2025-05-09.09-19-59.webm

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

Labels

2. developing bug Something isn't working regression

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Add new text will jump randomly to a comment field of a picture Loading an existing file focusses image description

6 participants