Skip to content

Conversation

@luka-nextcloud
Copy link
Contributor

@luka-nextcloud luka-nextcloud commented Sep 9, 2025

๐Ÿ“ Summary

Add close button when open read-only files on mobile app

๐Ÿ–ผ๏ธ Screenshots

๐Ÿš๏ธ Before ๐Ÿก After
image image

๐Ÿ 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

@codecov
Copy link

codecov bot commented Sep 9, 2025

Codecov Report

โœ… All modified and coverable lines are covered by tests.
โœ… Project coverage is 59.54%. Comparing base (ea23129) to head (9526394).
โš ๏ธ Report is 22 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #7624   +/-   ##
=======================================
  Coverage   59.54%   59.54%           
=======================================
  Files         310      310           
  Lines       38748    38754    +6     
  Branches      937      937           
=======================================
+ Hits        23072    23078    +6     
  Misses      15568    15568           
  Partials      108      108           

โ˜” 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
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

Thanks for tackling this! I didn't test yet, just reviewed the code changes.

@luka-nextcloud luka-nextcloud force-pushed the add-close-button-on-readonly branch from 021522c to 8b770e2 Compare September 11, 2025 09:37
@luka-nextcloud luka-nextcloud added bug Something isn't working 3. to review labels Sep 11, 2025
@luka-nextcloud luka-nextcloud moved this to ๐Ÿ‘€ In review in ๐Ÿ“ Office team Sep 11, 2025
@luka-nextcloud luka-nextcloud force-pushed the add-close-button-on-readonly branch from 8b770e2 to 00c720c Compare September 16, 2025 11:34
Copy link
Member

@mejo- mejo- left a comment

Choose a reason for hiding this comment

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

I just now took a deeper look. I think there's no need to add the button to the Editor.vue component manually. Instead, you should make sure that the #header template from DirectEditing.vue is added to the <ReadOnlyBar> element in Editor.vue just like it's done for <MenuBar>.

While testing this, I realized that the ReadOnlyBar doesn't display well on mobile (at the bottom), it's cut off and the border line is on the wrong side. If you could take a look into improving this and harmonizing it with how MenuBar is displayed on mobile, that would be awesome. If not we can also do it in a follow-up PR. (already tackled in #7560)

@max-nextcloud
Copy link
Collaborator

#7560 seems related.

@mejo-
Copy link
Member

mejo- commented Sep 17, 2025

#7560 seems related.

Ah wow, I missed that. It seems to address my second comment from above ๐Ÿ™ Will take a look.

@mejo-
Copy link
Member

mejo- commented Sep 17, 2025

While testing this, I realized that the ReadOnlyBar doesn't display well on mobile (at the bottom), it's cut off and the border line is on the wrong side. If you could take a look into improving this and harmonizing it with how MenuBar is displayed on mobile, that would be awesome. If not we can also do it in a follow-up PR.

So this is no longer necessary as it got tackled in #7560 already ๐Ÿ™

@luka-nextcloud luka-nextcloud force-pushed the add-close-button-on-readonly branch from 00c720c to 9526394 Compare September 18, 2025 12:01
@mejo- mejo- merged commit 158fa49 into main Sep 22, 2025
66 checks passed
@mejo- mejo- deleted the add-close-button-on-readonly branch September 22, 2025 09:16
@github-project-automation github-project-automation bot moved this from ๐Ÿ‘€ In review to โ˜‘๏ธ Done in ๐Ÿ“ Office team Sep 22, 2025
@mejo-
Copy link
Member

mejo- commented Sep 22, 2025

/backport to stable32

@mejo-
Copy link
Member

mejo- commented Sep 22, 2025

/backport to stable30

@mejo-
Copy link
Member

mejo- commented Sep 22, 2025

/backport to stable31

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

Labels

3. to review bug Something isn't working

Projects

Archived in project

Development

Successfully merging this pull request may close these issues.

Close button does not show when opening readonly .txt or .md files in iOS and Android Nextcloud app

4 participants