Skip to content

Conversation

@skjnldsv
Copy link
Member

@skjnldsv skjnldsv commented Jul 8, 2025

Might require nextcloud/server#53872

Master have received a few fixes, the current workspace implementation wasn't optimal, so I fixed it.

  1. Now, the headers are properly ALWAYS mounted
  2. We don't need to keep track of the newWorkspaceCreated
  3. You can emit a proper files:node:updated to dispatch the fact that the current folder have changed (and update the workspace attributes accordingly)

@codecov
Copy link

codecov bot commented Jul 8, 2025

Codecov Report

Attention: Patch coverage is 92.85714% with 2 lines in your changes missing coverage. Please review.

Project coverage is 52.61%. Comparing base (9ea5752) to head (cb3235b).
Report is 3 commits behind head on main.

Files with missing lines Patch % Lines
src/views/RichWorkspace.vue 0.00% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #7418      +/-   ##
==========================================
- Coverage   52.63%   52.61%   -0.02%     
==========================================
  Files         494      494              
  Lines       42296    42284      -12     
  Branches     1088     1088              
==========================================
- Hits        22261    22248      -13     
- Misses      19928    19929       +1     
  Partials      107      107              

☔ 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.

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.

Two minor nitpicks - other than that looks great. Thanks for taking care of this so quickly!
❤️

@skjnldsv
Copy link
Member Author

skjnldsv commented Jul 9, 2025

Done @max-nextcloud 👍

@max-nextcloud max-nextcloud merged commit e6c3197 into main Jul 9, 2025
77 of 79 checks passed
@max-nextcloud max-nextcloud deleted the fix/workspace branch July 9, 2025 20:21
@max-nextcloud
Copy link
Collaborator

/backport to stable31

@max-nextcloud
Copy link
Collaborator

Testing out if this helps with rich workspace tests failing in stable31 lately.

@skjnldsv
Copy link
Member Author

@max-nextcloud seems like when you load an existing workspace, clicking the content doesn't trigger the editor mode anymore 🤔

@max-nextcloud
Copy link
Collaborator

@skjnldsv I'll take a look. Thanks for the heads up.

@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Jul 14, 2025

grafik Seeing this in the console makes me think we do not extend the Vue instance with the l10n functions.

update adding them back removes the error but does not change the behavior.

@skjnldsv
Copy link
Member Author

Ah, nice catch!
Extending the vue with globals is a deprecated behaviour anyway.
I'd suggest always importing on each component to be cleaner :)

@max-nextcloud
Copy link
Collaborator

max-nextcloud commented Jul 14, 2025

I suspect this broke due to a later change in server. Otherwise i don't see how the cypress tests would have passed as they test editing the workspace.

update: most cypress tests pass locally for me as they create the workspace and then test it. - and that works fine.

💭 thinking out loud
It looks like only the RichTextReader is getting mounted because this.getFileInfo is not getting called as this.hasRichWorkspace is false during onMounted.

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

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants