fix/move wide page toggle #7514

max-nextcloud · 2025-07-29T12:31:59Z

chore(extract): TranslateButton from MenuBar
chore(move): handleEditorWidthChange into SessionList
chore(npm): prettier:change to format only changed files
fix(width): load initial state in editor
fix(width): remember full width on remount
fix(width): use nextcloud event bus to sync width
fix(width): move toggle into remaining actions menu

codecov · 2025-07-29T12:51:31Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 59.92%. Comparing base (b874cae) to head (2050ce8).
⚠️ Report is 20 commits behind head on main.

Additional details and impacted files

@@             Coverage Diff             @@
##             main    #7514       +/-   ##
===========================================
+ Coverage   46.65%   59.92%   +13.27%     
===========================================
  Files         498      500        +2     
  Lines       42153    38218     -3935     
  Branches     1160     1140       -20     
===========================================
+ Hits        19665    22904     +3239     
+ Misses      22370    15206     -7164     
+ Partials      118      108       -10

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

Thanks for tackling this. I found some minor issues and have some questions 😆

src/components/Menu/MenuBar.vue

src/components/Menu/TranslateButton.vue

package.json

src/composables/useEditorWidth.ts

src/components/Menu/WidthToggle.vue

src/composables/useEditorWidth.ts

src/event-bus.d.ts

src/components/Menu/MenuBar.vue

src/components/Editor/SessionList.vue

max-nextcloud · 2025-08-03T20:14:26Z

The only thing that's missing from also fixing #6703 would be hiding the toggle in collectives as collectives has its own toggle. I'm unsure how best to achieve this though.

We could hide it everywhere where createEditor is used - but that seems too generic.
Collectives could specify a parameter to createEditor.
Collectives could provide some data that we inject in the Editor that hides the toggle.

Alternatively we could make the toggle work on the collectives per page setting - but I wonder if that would be more confusing or less.

update: I found a different way (f8ee194):
Text now detects the width has already been set and does not render the width toggle anymore.

Signed-off-by: Max <[email protected]>

Next steps: * split it to a separate component. * move it into the remaining actions menu item. Signed-off-by: Max <[email protected]>

This way the editor width is adjusted before the session list loads. Signed-off-by: Max <[email protected]>

After closing and reopening the editor the full width value was read from initial state again. Use a singleton instead so the toggled state is persisted. Signed-off-by: Max <[email protected]>

If multiple editors are open (i.e. folder description and viewer) set the full width value on both of them using the event bus. Signed-off-by: Max <[email protected]>

Signed-off-by: Max <[email protected]>

The component is used inside NcActions now. Signed-off-by: Max <[email protected]>

Signed-off-by: Max <[email protected]>

mejo- · 2025-08-12T08:38:21Z

src/composables/useEditorWidth.ts

+	const setByText = document.documentElement.style.getPropertyValue(
+		'--text-editor-max-width',
+	)
+	return alreadySet && alreadySet !== setByText


I fear I don't fully understand the logic here. Both read the value of the css variable --text-editor-max-width, the first from the <body> element, the second from the root element (<html>). So Text attaches the variable to the root <html> element and Collectives attaches it to the <body> element and that's how you distinguish them?

In my tests with latest master branch of server, the latter is always empty, even if I open a text editor from the Files app. In other words I never see the full width toggle at the moment.

In general, this feels a bit brittle to me as it will make the fullWidth toggle disappear as soon as we change something with how --text-editor-max-width is set.

Maybe it's really better to be more explicit and make this configurable via the editorApi at editor.js?

I just checked the code and it seems to me that actually both Text and Collectives set --text-editor-max-width on the root element: see https://github.com/nextcloud/collectives/blob/aeffe87608250a50d0eb99b526362616fa2e0eb2/src/css/editor.scss#L7 and

text/src/css/variables.scss

Line 7 in c74d204

--text-editor-max-width: 80ch;

Also I don't understand why you use getComputedProperties(<element>) first and <element>.style second. Is this intended?

What I am trying to do here is this:

Read the effective value of --text-editor-max-width for the document body. I'm using getComputedStyle so it does not matter where exactly the style is set - wether on the body element or :root or in some other way.

Check if text has set a value already - for example because it was opened before. This value would always be set on document.documentElement.style (as implemented below).

If 1. and 2. both have a value set - check if it's the same to determine if 1 is only set because of 2.

Now let me figure out why this is not working as intended.

Okay... I see what's happening here. Text also set's the width on :root - twice in fact. So 1 returns a value even when 2 has not been set yet.

Text set the --text-editor-max-width in css. I removed that as it will be set either externally or by useEditorWidth now.

mejo-

As discussed this PR is good to go from my side with two minor comments. One I put in the code, the other would be that a Cypress test would be great to protect against regresssions.

src/composables/useEditorWidth.ts

The collectives app has its own mechanism for setting the desired editor width. Detect the custom width being set and do not render the width toggle. Signed-off-by: Max <[email protected]>

This way it is easier to test and all the translation logic is in one place. Signed-off-by: Max <[email protected]>

Signed-off-by: Max <[email protected]>

max-nextcloud requested a review from mejo- as a code owner July 29, 2025 12:32

max-nextcloud force-pushed the fix/move-wide-page-toggle branch 2 times, most recently from a913266 to bcb708b Compare July 30, 2025 07:14

mejo- requested changes Jul 30, 2025

View reviewed changes

max-nextcloud mentioned this pull request Mar 3, 2025

Line length toggle follow ups #6703

Closed

2 tasks

max-nextcloud force-pushed the fix/move-wide-page-toggle branch from bcb708b to f8ee194 Compare August 6, 2025 08:25

max-nextcloud mentioned this pull request Aug 6, 2025

feat(status): Display offline state instead of session list #7420

Merged

6 tasks

max-nextcloud force-pushed the fix/move-wide-page-toggle branch from fc0d75d to becbd1d Compare August 6, 2025 10:14

max-nextcloud requested a review from mejo- August 11, 2025 19:37

max-nextcloud added 11 commits August 12, 2025 10:09

chore(extract): TranslateButton from MenuBar

b236a59

Signed-off-by: Max <[email protected]>

chore(move): handleEditorWidthChange into SessionList

fad762d

Next steps: * split it to a separate component. * move it into the remaining actions menu item. Signed-off-by: Max <[email protected]>

fix(width): load initial state in editor

e094c15

This way the editor width is adjusted before the session list loads. Signed-off-by: Max <[email protected]>

fix(width): remember full width on remount

9a67122

After closing and reopening the editor the full width value was read from initial state again. Use a singleton instead so the toggled state is persisted. Signed-off-by: Max <[email protected]>

fix(width): use nextcloud event bus to sync width

4f34935

If multiple editors are open (i.e. folder description and viewer) set the full width value on both of them using the event bus. Signed-off-by: Max <[email protected]>

fix(width): move toggle into remaining actions menu

9d60abc

Signed-off-by: Max <[email protected]>

chore(cleanup): old console.debug

d0646dc

Signed-off-by: Max <[email protected]>

chore(lint): use arrow syntax for internal function

4571584

Signed-off-by: Max <[email protected]>

fix(focus-trap): mark session list popover as no focus trap

3a7eb56

Signed-off-by: Max <[email protected]>

fix(WidthToggle): use NcActionCheckbox

ec575ff

The component is used inside NcActions now. Signed-off-by: Max <[email protected]>

chore(cleanup): suprefluous redrawEditor after width toggle

55d4c65

Signed-off-by: Max <[email protected]>

max-nextcloud force-pushed the fix/move-wide-page-toggle branch from becbd1d to 9804c10 Compare August 12, 2025 08:09

mejo- requested changes Aug 12, 2025

View reviewed changes

max-nextcloud force-pushed the fix/move-wide-page-toggle branch from d9b7cbe to b76ac59 Compare August 12, 2025 12:32

max-nextcloud requested a review from mejo- August 12, 2025 12:33

max-nextcloud force-pushed the fix/move-wide-page-toggle branch from b76ac59 to 8f1c46b Compare August 12, 2025 18:37

mejo- approved these changes Aug 13, 2025

View reviewed changes

src/composables/useEditorWidth.ts Show resolved Hide resolved

max-nextcloud added 2 commits August 13, 2025 15:21

fix(menu): no width toggle if width is set externally

8b12424

The collectives app has its own mechanism for setting the desired editor width. Detect the custom width being set and do not render the width toggle. Signed-off-by: Max <[email protected]>

chore(move): canTranslate into the TranslateButton

dbfc23c

This way it is easier to test and all the translation logic is in one place. Signed-off-by: Max <[email protected]>

max-nextcloud added 3 commits August 13, 2025 15:21

fix(guest): enable focus trap for guest name entry

c8b573e

Signed-off-by: Max <[email protected]>

chore(docs): explain --text-editor-max-width logic

cbe1383

Signed-off-by: Max <[email protected]>

chore(tests): e2e test editor width toggle

2050ce8

Signed-off-by: Max <[email protected]>

max-nextcloud force-pushed the fix/move-wide-page-toggle branch from 8f1c46b to 2050ce8 Compare August 13, 2025 20:46

max-nextcloud merged commit c36e12a into main Aug 13, 2025
66 checks passed

max-nextcloud deleted the fix/move-wide-page-toggle branch August 13, 2025 21:21

skjnldsv mentioned this pull request Aug 19, 2025

32.0.0 beta 1 nextcloud/server#54502

Merged

mejo- assigned max-nextcloud Dec 13, 2025

fix/move wide page toggle #7514

fix/move wide page toggle #7514

Uh oh!

Conversation

max-nextcloud commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jul 29, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

mejo- left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

max-nextcloud commented Aug 3, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mejo- Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

mejo- Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

mejo- Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

max-nextcloud Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

max-nextcloud Aug 12, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

max-nextcloud Aug 12, 2025

Choose a reason for hiding this comment

Uh oh!

mejo- left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

max-nextcloud commented Jul 29, 2025 •

edited

Loading

codecov bot commented Jul 29, 2025 •

edited

Loading

max-nextcloud commented Aug 3, 2025 •

edited

Loading

max-nextcloud Aug 12, 2025 •

edited

Loading

max-nextcloud Aug 12, 2025 •

edited

Loading