Skip to content

Conversation

@vinicius73
Copy link
Member

Readonly

Peek.2022-09-14.18-03.mp4

Public

Peek.2022-09-14.17-01.mp4

@vinicius73 vinicius73 added enhancement New feature or request design Experience, interaction, interface, … 3. to review labels Sep 14, 2022
@vinicius73 vinicius73 added this to the Nextcloud 25 milestone Sep 14, 2022
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.

Code looks good.

However if i click an entry in the outline it does not move to that heading. That's not what i'd expect because the entry is highlighted and looks like a link to me.

I also managed to trigger some funny behavior when clicking at the end of headings. I was not able to produce the same behavior on the editable view of the owner of the file.
Bildschirmaufzeichnung vom 15.09.2022, 08:19:51.webm

Copy link
Member

@jancborchardt jancborchardt left a comment

Choose a reason for hiding this comment

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

Nice! Another point in addition to @susnux:
The "x" close button is a bit floating in the air. Would be better if it’s directly after the "Outline" text instead of floating on the right.

@mejo-
Copy link
Member

mejo- commented Sep 15, 2022

Thanks a lot for looking into this @vinicius73 ❤️

Unfortunately using the menubar space in read-only mode conflicts with Collectives, where we already use this space for Collectives-specific content (last changed user and timestamp, in future also things like count of backlinks and attachments). Sorry, I don't have a quick solution for this. Maybe we should discuss it in our next team call?

@vinicius73
Copy link
Member Author

Thanks a lot for looking into this @vinicius73 ❤️

Unfortunately using the menubar space in read-only mode conflicts with Collectives, where we already use this space for Collectives-specific content (last changed user and timestamp, in future also things like count of backlinks and attachments). Sorry, I don't have a quick solution for this. Maybe we should discuss it in our next team call?

What do you think about make able to inject a slot that space and expose some editor methods?

So that way you will be able to not only inject more options also access internal editor methods in a safe way

@vinicius73
Copy link
Member Author

@jancborchardt what do you think about this?

image

@mejo-
Copy link
Member

mejo- commented Sep 19, 2022

What do you think about make able to inject a slot that space and expose some editor methods?

So that way you will be able to not only inject more options also access internal editor methods in a safe way

That would be the way to go indeed 😊

@vinicius73 vinicius73 force-pushed the 2904-table-of-contents-add-button-to-close-outline-from-itself branch 2 times, most recently from e8c7382 to 76fbc1e Compare September 21, 2022 14:52
@jancborchardt
Copy link
Member

@vinicius73 it's better if the x is on the right if the text, so that the text is left-aligned with the headings below. :) Just not all the way over away from the text.

@blizzz blizzz mentioned this pull request Sep 22, 2022
2 tasks
@juliusknorr
Copy link
Member

/backport to stable25

@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 22, 2022
@mejo- mejo- modified the milestones: Nextcloud 26, Nextcloud 25 Sep 27, 2022
@blizzz blizzz modified the milestones: Nextcloud 25, Nextcloud 26 Sep 27, 2022
@blizzz
Copy link
Member

blizzz commented Sep 27, 2022

master is 26

@mejo-
Copy link
Member

mejo- commented Oct 10, 2022

@vinicius73 could you take a quick look into rebasing the PR on latest master? Seems like something went wrong and unrelated commits got pulled in 😉

@vinicius73 vinicius73 force-pushed the 2904-table-of-contents-add-button-to-close-outline-from-itself branch from 34d4a6b to 230473d Compare October 11, 2022 15:23
@vinicius73
Copy link
Member Author

@mejo- I've rewritten the all PR, check if you can please

@vinicius73 vinicius73 force-pushed the 2904-table-of-contents-add-button-to-close-outline-from-itself branch from 230473d to 103d8ef Compare October 11, 2022 15:34
@mejo-
Copy link
Member

mejo- commented Oct 11, 2022

Nice, thanks a lot @vinicius73. Works like expected. I removed the top: 104px as discussed here. It doesn't solve the problem for Collectives yet, but at least it doesn't break Collectives any longer - and it adds outline support for read-only Text shares, which is great 😍

@mejo-
Copy link
Member

mejo- commented Oct 11, 2022

/compile

Signed-off-by: nextcloud-command <[email protected]>
@mejo- mejo- merged commit c751073 into master Oct 11, 2022
@delete-merged-branch delete-merged-branch bot deleted the 2904-table-of-contents-add-button-to-close-outline-from-itself branch October 11, 2022 19:46
@backportbot-nextcloud
Copy link

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

@mejo-
Copy link
Member

mejo- commented Oct 11, 2022

/backport 103d8ef,7d9a18b15c56608fa38492ec9d7f877130922488 to stable25

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

Labels

3. to review design Experience, interaction, interface, … enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Table of contents: add button to close outline from itself

9 participants