Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Bring back menubar without formatting on plain text mode #2757

Merged
merged 3 commits into from
Nov 16, 2022
Merged

Bring back menubar without formatting on plain text mode #2757

merged 3 commits into from
Nov 16, 2022

Conversation

juliusknorr
Copy link
Member

@juliusknorr juliusknorr commented Jul 27, 2022 •
edited
Loading

Fixes #2625

  • Ensure that the session list on the right of the menubar is always visible
    • For plain text files
    • For read only shares of markdown and plain text files

@juliusknorr juliusknorr force-pushed the bugfix/noid/menubar branch from df061f3 to 5f05fb1 Compare July 27, 2022 18:05
@juliusknorr juliusknorr added bug Something isn't working 3. to review labels Jul 27, 2022
@nextcloud-command nextcloud-command force-pushed the refactor/split-editor-wrapper branch from 9adc0e0 to 44ad290 Compare July 27, 2022 21:44
@max-nextcloud max-nextcloud force-pushed the refactor/split-editor-wrapper branch from 44ad290 to 4f58e4c Compare July 29, 2022 12:20
@delete-merged-branch delete-merged-branch bot deleted the branch master July 29, 2022 14:01
@juliusknorr juliusknorr force-pushed the bugfix/noid/menubar branch from 5f05fb1 to 7b24b8a Compare August 1, 2022 20:03
@juliusknorr juliusknorr changed the base branch from refactor/split-editor-wrapper to master August 1, 2022 20:04
@juliusknorr
Copy link
Member Author

Rebased and ready for review :)

max-nextcloud
max-nextcloud previously requested changes Aug 2, 2022
View reviewed changes
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.

All in all looks good. I think we do not need to render the menu bubble unless we are editing a richtext document.
With the change to renderMenus it would now be rendered for text. Since it even has it's own dynamic import might be worthwhile having two computed conditions renderMenubar and renderMenububble or so.

@juliusknorr juliusknorr added this to the Nextcloud 26 milestone Oct 11, 2022
@juliusknorr
Copy link
Member Author

Adjusted the behavior a bit to also bring back the menu bar items on read only files.

We probably could refactor the menubar (with regards to the Status component) a bit to not pass all the individual props twice but I kept the changes smaller in order to still have this backportable for now.

@juliusknorr juliusknorr requested a review from a team October 27, 2022 15:28
@juliusknorr
Copy link
Member Author

Another conflict, will look into that

@juliusknorr
Copy link
Member Author

Rebased and ready for review again :)

@cypress
Copy link

cypress bot commented Nov 15, 2022 •
edited
Loading


Test summary

106 • 0 • 0 • 0 • Flakiness 2

Run details
Project Text
Status Passed
Commit 5db37d1 ℹ️
Started Nov 16, 2022 7:54 AM
Ended Nov 16, 2022 8:18 AM
Duration 24:32 💡
OS Linux Ubuntu -
Browser Electron 106

View run in Cypress Dashboard ➡️

Flakiness

links.spec.js 1 Flakiness
1 test link marks > link preview > shows a link preview
sections.spec.js 1 Flakiness
1 Content Sections > Heading anchors > Anchor scrolls into view

This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard

@juliusknorr juliusknorr mentioned this pull request Nov 15, 2022
Closed
@juliusknorr @max-nextcloud
Bring back menubar without formatting on plain text mode
6657642 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@juliusknorr @max-nextcloud
Fix odd focus on session list trigger (fixes #2731)
ebc05a5 
Signed-off-by: Julius Härtl <jus@bitgrid.net>
@max-nextcloud
Copy link
Collaborator

/compile

@nextcloud-command
Compile assets
306b920 
Signed-off-by: nextcloud-command <nextcloud-command@users.noreply.github.com>
max-nextcloud
max-nextcloud approved these changes Nov 16, 2022
View reviewed changes
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.

Looks good. Just one function that i think is not needed anymore. But that should not block a merge.

src/components/Editor.vue Show resolved Hide resolved
@max-nextcloud
Copy link
Collaborator

cypress was failing earlier - curious to see if this still happens.

@max-nextcloud
Copy link
Collaborator

/backport to stable25

@max-nextcloud
Copy link
Collaborator

/backport to stable24

@max-nextcloud
Copy link
Collaborator

Not sure if a backport to 23 is needed. We need the one to stable 24 for sure as a customer complained about this (in ios but i think it's this issue.)

@max-nextcloud
Copy link
Collaborator

Looks like for the keyboard shortcut tests retries run into file version conflicts

@max-nextcloud max-nextcloud mentioned this pull request Nov 16, 2022
Closed
@max-nextcloud max-nextcloud merged commit 84be752 into master Nov 16, 2022
@delete-merged-branch delete-merged-branch bot deleted the bugfix/noid/menubar branch November 16, 2022 08:52
@backportbot-nextcloud backportbot-nextcloud bot mentioned this pull request Nov 16, 2022
Merged
1 task
@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

@max-nextcloud
Copy link
Collaborator

/backport 6657642 to stable24

@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

max-nextcloud added a commit that referenced this pull request Nov 23, 2022
@max-nextcloud
fix: menubar without formatting on plain text mode
4b49f3e 
Not really a backport but rather a reimplementation of #2757.

Signed-off-by: Max <max@nextcloud.com>
@max-nextcloud max-nextcloud mentioned this pull request Nov 23, 2022
Merged
4 tasks
@blizzz blizzz mentioned this pull request Feb 9, 2023
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@max-nextcloud max-nextcloud max-nextcloud approved these changes

@vinicius73 vinicius73 Awaiting requested review from vinicius73

@luka-nextcloud luka-nextcloud Awaiting requested review from luka-nextcloud

Assignees
No one assigned
Labels
3. to review bug Something isn't working
Projects
None yet
Milestone
Nextcloud 26
Development

Successfully merging this pull request may close these issues.

Doc saved indicator and user color controls are missing for .txt files
3 participants
@juliusknorr @max-nextcloud @nextcloud-command