Skip to content

Refactor document status and menubar padding - #7130

Merged
mejo- merged 10 commits into
mainfrom
fix/refactor_document_status
Apr 30, 2025
Merged

Refactor document status and menubar padding#7130
mejo- merged 10 commits into
mainfrom
fix/refactor_document_status

Conversation

@mejo-

Copy link
Copy Markdown
Member

📝 Summary

  • fix: Make CollisionResolveDialog and DocumentStatus sticky
  • fix: Remove overflow property from editor wrapper
  • refactor(css): Use default-grid-baseline for menubar padding
  • fix(Editor): Add property lock to data
  • test(cy): Add regression test for sticky elements

🏁 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

@mejo-mejo- added bug Something isn't working 3. to review regression labels Apr 16, 2025
@mejo-mejo- self-assigned this Apr 16, 2025
@github-project-automationgithub-project-automationBot moved this to 🧭 Planning evaluation (don't pick) in 📝 Productivity teamApr 16, 2025
@codecov

codecovBot commented Apr 16, 2025

Copy link
Copy Markdown

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.85%. Comparing base (95160cd) to head (87f2ddf).
Report is 11 commits behind head on main.

Additional details and impacted files
@@ Coverage Diff @@## main #7130 +/- ##
==========================================
+ Coverage 51.83% 51.85% +0.02% 
==========================================
Files 479 479 Lines 41488 41510 +22 Branches 1002 1002 ==========================================
+ Hits 21504 21526 +22 
Misses 19879 19879 Partials 105 105 

☔ 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-
mejo-force-pushed the fix/refactor_document_status branch from 0ff0401 to 8d43fb1CompareApril 16, 2025 11:42
@mejo-
mejo- requested a review from juliusknorrApril 16, 2025 12:09
@mejo-mejo- moved this from 🧭 Planning evaluation (don't pick) to 👀 In review in 📝 Productivity teamApr 16, 2025
@mejo-

Copy link
Copy Markdown
MemberAuthor

/backport to stable31

@mejo-
mejo-force-pushed the fix/refactor_document_status branch from 8d43fb1 to cc5fef4CompareApril 29, 2025 12:47
mejo- added 2 commits April 29, 2025 14:48
Move DocumentStatus into parent container to do so.
Fixes stickyness of document status if editor does not have the full
app content height, e.g. in Collectives or Deck.
Signed-off-by: Jonas <jonas@freesources.org>
Fixes stickyness of menubar when editor does not have the full app
height, e.g. in Collectives and Deck.
Fixes: nextcloud/collectives#1704
Signed-off-by: Jonas <jonas@freesources.org>
@mejo-
mejo-force-pushed the fix/refactor_document_status branch from cc5fef4 to e7d5527CompareApril 29, 2025 12:48
mejo- added 3 commits April 29, 2025 14:54
Signed-off-by: Jonas <jonas@freesources.org>
Fixes "[Vue warn]: Property or method "lock" is not defined on the
instance but referenced during render."
Signed-off-by: Jonas <jonas@freesources.org>
Make sure that document status and collision resolve dialog stay visible
when scrolling down long documents.
Signed-off-by: Jonas <jonas@freesources.org>
@mejo-
mejo-force-pushed the fix/refactor_document_status branch from e7d5527 to e270905CompareApril 29, 2025 12:54
@juliusknorrjuliusknorr changed the title Refactor document status and menbar paddingRefactor document status and menubar paddingApr 29, 2025
…overflow
Signed-off-by: Jonas <jonas@freesources.org>
@mejo-

mejo- commented Apr 29, 2025

Copy link
Copy Markdown
MemberAuthor

I again did extensive testing to find possible regressions and added another fix concerning mobile view with menubar at the bottom.

This is the test matrix I went through:

✔️ = no problems
❌✅ = problem, but fixed now

appviewcontentdocument statuscollisionstatus
collectivesdesktoplongnono✔️
collectivesdesktoplongyesno✔️
collectivesdesktoplongyesyes✔️
collectivesdesktopshortnono✔️
collectivesdesktopshortyesno✔️
collectivesdesktopshortyesyes✔️
collectivesmobilelongnono❌✅
collectivesmobilelongyesno✔️
collectivesmobilelongyesyes✔️
collectivesmobileshortnono✔️
collectivesmobileshortyesno✔️
collectivesmobileshortyesyes✔️
collectives (landingpage)desktoplongnono✔️
collectives (landingpage)desktopshortnono✔️
collectives (landingpage)mobilelongyesyes❌✅
collectives (landingpage)mobileshortyesyes❌✅
standalonedesktoplongnono✔️
standalonedesktoplongyesno❌✅
standalonedesktoplongyesyes❌✅
standalonedesktopshortnono✔️
standalonedesktopshortyesno✔️
standalonedesktopshortyesyes✔️
standalonemobilelongnono✔️
standalonemobilelongyesno✔️
standalonemobilelongyesyes❌✅
standalonemobileshortnono✔️
standalonemobileshortyesno✔️
standalonemobileshortyesyes✔️

@max-nextcloud

Copy link
Copy Markdown
Collaborator

Scrollbar on mobile

I'm now seeing a scrollbar on short documents in chromium and if a message is displayed the scrollable area becomes higher:

Bildschirmaufzeichnung.vom.2025-04-30.08-56-34.mp4

Positioning on standalone (Chromium)

(Not sure this is an issue...)
With a short document the status now shows in the middle of the doc:

Bildschirmfoto vom 2025-04-30 08-48-30

mejo- added 4 commits April 30, 2025 11:03
Use flexbox layout instead of height `100%` to avoid issues where editor
is embedded into other apps and doesn't take full height.
Signed-off-by: Jonas <jonas@freesources.org>
Signed-off-by: Jonas <jonas@freesources.org>
We want the border to separate document content and menubar. So when we
display the menubar at the bottom on mobile, the menubar border should
be on top, not bottom of it.
Signed-off-by: Jonas <jonas@freesources.org>
I spent some time trying to figure out where the extra 8px came from
that created the scrollbar even with short content on mobile. In the end
this seems like an acceptable workaround for now.
Signed-off-by: Jonas <jonas@freesources.org>
@mejo-
mejo-force-pushed the fix/refactor_document_status branch from a6c93ef to 87f2ddfCompareApril 30, 2025 11:55
@mejo-

Copy link
Copy Markdown
MemberAuthor

With a short document the status now shows in the middle of the doc:

Fixed this by using flexbox layout and flex-grow: 1.

I'm now seeing a scrollbar on short documents in chromium and if a message is displayed the scrollable area becomes higher:

Both also fixed now.

@mejo-
mejo- merged commit 7392881 into mainApr 30, 2025
@mejo-
mejo- deleted the fix/refactor_document_status branch April 30, 2025 15:59
@github-project-automationgithub-project-automationBot moved this from 👀 In review to ☑️ Done in 📝 Productivity teamApr 30, 2025
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3. to reviewbugSomething isn't workingregression

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

2 participants

@mejo-@max-nextcloud