Skip to content

Comments

Fix anchcors scrolling#1150

Merged
mejo- merged 1 commit intonextcloud:mainfrom
Koc:feature/fix-anchcors-scrolling
Mar 25, 2024
Merged

Fix anchcors scrolling#1150
mejo- merged 1 commit intonextcloud:mainfrom
Koc:feature/fix-anchcors-scrolling

Conversation

@Koc
Copy link
Contributor

@Koc Koc commented Mar 9, 2024 •
edited
Loading

📝 Summary

Steps to reproduce:

  • Create long page with multiple headings
  • Click on heading in the bottom of the page
  • Copy url from address bar with anchor and open it in new tab

Expected result:

  • Content scrolled to heading

Actual result:

  • There are no scroll

🖼️ Screenshots

🏚️ Before 🏡 After
image image

🚧 TODO

🏁 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

@Koc Koc force-pushed the feature/fix-anchcors-scrolling branch from 2889952 to bda1252 Compare March 10, 2024 22:27
@juliusknorr juliusknorr requested a review from mejo- March 11, 2024 05:59
@juliusknorr juliusknorr added bug Something isn't working 3. to review labels Mar 11, 2024
@juliusknorr juliusknorr self-requested a review March 11, 2024 05:59
@mejo- mejo- force-pushed the feature/fix-anchcors-scrolling branch from bda1252 to 1dcd676 Compare March 11, 2024 19:07
@mejo- mejo- mentioned this pull request Mar 14, 2024
Merged
2 tasks
@mejo-
Copy link
Member

mejo- commented Mar 14, 2024

For some reason onLoaded callback is not called in my case, so I'm using setTimeout. Any idea how to fix that?

Currently only the session editor (Editor.vue in the text app) supports the onLoaded callback. But you're right, the editor used for the view mode (MarkdownContentEditor in the text app) should do so as well. I opened nextcloud/text#5489 to implement this.

mejo-
mejo- requested changes Mar 14, 2024
View reviewed changes
@Koc Koc force-pushed the feature/fix-anchcors-scrolling branch from 1dcd676 to c45360f Compare March 21, 2024 15:10
@Koc Koc mentioned this pull request Mar 21, 2024
Closed
4 tasks
@Koc
Copy link
Contributor Author

Koc commented Mar 21, 2024

@mejo- I've updated current PR, and opened one more to nextcloud-vue. Please review one more time.

mejo-
mejo- requested changes Mar 21, 2024
View reviewed changes
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.

Dear @Koc, great to hear that onLoaded works now 😊

Regarding your PR against nextcloud-vue, I think that we should rather change the behaviour here in Collectives. It's not given that the contents given to NcAppContent contain a heading like in collectives. Adding something like the following to Collectives.vue should do the trick:

:deep(.splitpanes__pane-details) {
	scroll-padding-top: 60px;
	scroll-behavior: smooth;
}

@github-actions
Copy link
Contributor

github-actions bot commented Mar 24, 2024

Hello there,
Thank you so much for taking the time and effort to create a pull request to our Nextcloud project.

We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process.

Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6

Thank you for contributing to Nextcloud and we hope to hear from you soon!

@Koc
Fix anchcors scrolling
0847e80 
Signed-off-by: Konstantin Myakshin <[email protected]>
@Koc Koc force-pushed the feature/fix-anchcors-scrolling branch from c45360f to 0847e80 Compare March 25, 2024 08:55
@Koc
Copy link
Contributor Author

Koc commented Mar 25, 2024

@mejo- thank you for suggestion!
I've pushed changes and closed related PR. The only one thing - I've omit :deep to be consistent with other styles. It works fine without it, as non-scopped styles used

mejo-
mejo- approved these changes Mar 25, 2024
View reviewed changes
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.

Awesome, works like a charm now. Thanks a lot for the contribution @Koc!

Only #1114 sometimes breaks the behaviour, but that's a separate issue.

@mejo- mejo- merged commit 01cc927 into nextcloud:main Mar 25, 2024
@Koc Koc deleted the feature/fix-anchcors-scrolling branch March 25, 2024 11:02
@Koc Koc restored the feature/fix-anchcors-scrolling branch June 10, 2024 16:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@mejo- mejo- mejo- approved these changes

@juliusknorr juliusknorr Awaiting requested review from juliusknorr

Assignees

No one assigned

Labels

3. to review bug Something isn't working feedback-requested

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@Koc @mejo- @juliusknorr