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.

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

Fix scroll position of main content when left sidebar requires scrolling to show active item #3175

Merged
merged 1 commit into from
Feb 3, 2025

Conversation

DamianMaslanka5
Copy link
Contributor

@DamianMaslanka5 DamianMaslanka5 commented Jan 30, 2025

Summary

Try to fix scroll position of main content when left sidebar requires scrolling to show active item

Also now the search bar is always visible, previously search bar and the first item from the sidebar was hidden after scrolling the main content.

You can see both issues on https://clickhouse.com/docs/en/optimize/skipping-indexes

I made minimum amount of changes which fixed described issues.

I tested:

  1. Scrolling the left sidebar menu to active item works correctly - https://clickhouse.com/docs/en/optimize/skipping-indexes
  2. Scrolling to headings works correctly - https://clickhouse.com/docs/en/sql-reference/functions/string-functions#trim

Before

before.mp4

After

after.mp4

…ing to show active item

Also now the search bar is always visible, previously search bar and the first item from the sidebar was hidden after scrolling the main content.

Issue with the scroll position can be easily reproduced by navigating "/docs/en/optimize/skipping-indexes"
@DamianMaslanka5 DamianMaslanka5 marked this pull request as ready for review February 2, 2025 21:11
@DamianMaslanka5 DamianMaslanka5 changed the title Try to fix scroll position of main content when left sidebar requires scrolling to show active item Fix scroll position of main content when left sidebar requires scrolling to show active item Feb 2, 2025
@DamianMaslanka5
Copy link
Contributor Author

@gingerwizard I thinks this is ready for a review now, I found some time browse the docs with these changes and didn't see any issues.

Copy link
Member

@Blargian Blargian 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 to me, thanks @DamianMaslanka5!

@Blargian Blargian merged commit 795cf5b into ClickHouse:main Feb 3, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants