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

doc engine: fix scroll bar for the content sidebar #1284

Merged
merged 9 commits into from
May 21, 2020

Conversation

iKBAHT
Copy link
Contributor

@iKBAHT iKBAHT commented May 12, 2020

Fixes #1198

@iKBAHT iKBAHT changed the title freeze buttons block doc engine: fix scroll bar for the content sidebar May 12, 2020
@shcheklein shcheklein temporarily deployed to dvc-landing-fix-scroll--bt87yn May 12, 2020 14:36 Inactive
@shcheklein
Copy link
Member

@iKBAHT thanks! great.

A few things I noticed so far:

  1. Katacoda button should not be part of the scroll I think

Screen Shot 2020-05-12 at 8 03 33 AM

  1. CONTENT - - should not be part of the scroll probably. It'll become a back to the top of the page. (related nav: create title entry in right CONTENTS ToC sidebar gatsby-theme-iterative#37 )

  2. Can we make buttons go right after the sidebar as it was before. The reason here is we need to keep them as visible as possible. Especially "Katacoda" if it's visible.

@shcheklein shcheklein temporarily deployed to dvc-landing-fix-scroll--bt87yn May 12, 2020 21:59 Inactive
@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 12, 2020

I implemented noticed things. Also, I decrease distance between buttons from 25px to 15px. Otherwise, buttons block take too many heights with Katacoda block.

@shcheklein
Copy link
Member

@iKBAHT looks great overall, one more question - right now when there are some items that are hidden - sometimes it is not obvious that they exist (scroll is hidden). I kinda like that scroll is hidden, but is there a way from a user perspective to see that there is something below and you can scroll it ...? what do you think?

@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 13, 2020

I think the best way, in this case, is to always show a scroll bar if it exists.

@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 14, 2020

I don't even know how else to show the user that there is something below

@shcheklein
Copy link
Member

@iKBAHT yeah, I was thinking to try some fade out and show scroll when you hover over it.

The problem with always showing it is that it looks like a border line in some cases (when it's almost full) :)

Anyways, not a big deal - let me know if it's easy to try those two things ... otherwise I think we cab merge it as is.

@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 14, 2020

Fade-out is not a common practice, users are familiar with scrolls. Also, fade-out takes an already limited amount of space.
A solution with scroll is very easy to implement.

@shcheklein shcheklein temporarily deployed to dvc-landing-fix-scroll--bt87yn May 14, 2020 13:04 Inactive
@shcheklein
Copy link
Member

A solution with scroll is very easy to implement.

yep, works for now

One more thing we need to think and take care of:

We need to scroll into view the highlighted element when we open a link like this (and screen is not large enough).

https://dvc-landing-fix-scroll--bt87yn.herokuapp.com/doc/command-reference/fetch#example-with-dependencies

or as we scroll the screen and some items are not visible

@shcheklein shcheklein temporarily deployed to dvc-landing-fix-scroll--bt87yn May 18, 2020 14:06 Inactive
@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 18, 2020

We need to scroll into view the highlighted element when we open a link like this

done

Also, during implementation, I found the new bug #1306

@jorgeorpinel
Copy link
Contributor

Also, during implementation, I found the new bug #1306

Probably the same as iterative/gatsby-theme-iterative#38 🙂

@shcheklein
Copy link
Member

Something funny is happening - when I open https://dvc-landing-fix-scroll--bt87yn.herokuapp.com/ and click docs I'm getting:

Screen Shot 2020-05-18 at 6 14 35 PM

Note - no Home entry, Content went under the menu

Copy link
Member

@shcheklein shcheklein left a comment

Choose a reason for hiding this comment

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

@iKBAHT please check the deployed version (https://dvc-landing-fix-scroll--bt87yn.herokuapp.com/) - all the navigation is quite broken for me.

@shcheklein shcheklein temporarily deployed to dvc-landing-fix-scroll--bt87yn May 19, 2020 10:04 Inactive
@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 19, 2020

Yes, it's my fail. I tested only when scroll exists.

@shcheklein
Copy link
Member

@shcheklein shcheklein temporarily deployed to dvc-landing-fix-scroll--bt87yn May 19, 2020 16:22 Inactive
@shcheklein
Copy link
Member

Looks like scroll is not always visible as expected in Firefox

Also getting this:
Screen Shot 2020-05-19 at 5 56 56 PM

(It hides the text)

In Safari - it always scrolls the page for me, even if mouse pointer located on top of the sidebar area.

Also, it does not scroll the selected item into view after navigate? Am I missing something?

@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 20, 2020

  • I wasn't aware of the need to maintain FF. But anyway FF doesn't support "force" render scrollbar.
  • In Safari - it always scrolls the page for me, even if mouse pointer located on top of the sidebar area.

It's standard behavior, chrome works the same way. I mean if block doesn't have it own scroll, then scroll goes to parent.

  • Also, it does not scroll the selected item into view after navigate? Am I missing something?

Can you please write steps to reproduce. Everything works for me.

@shcheklein
Copy link
Member

I mean if block doesn't have it own scroll, then scroll goes to parent.

In this case it looks like it happens even if block has its own scroll.

I wasn't aware of the need to maintain FF.

yes, we support all major browsers + iOS Safari + Android

Can you please write steps to reproduce. Everything works for me.

may be we talk about different problem. I mean that an item on the right sidebar should be scrolled into view (same as we do with the left sidebar).

@iKBAHT
Copy link
Contributor Author

iKBAHT commented May 21, 2020

In this case it looks like it happens even if block has its own scroll.

Everything works for me. Can you share the problem somehow.

may be we talk about different problem. I mean that an item on the right sidebar should be scrolled into view (same as we do with the left sidebar).

I think it impossible. Right and left sidebars control by one scrollbar. And if scroll goes to the last article, and it short, the left bar also goes down with the whole page. It comes from main goal of this PR - "fix scroll bar for the content sidebar"

@shcheklein
Copy link
Member

Okay 👌 Merging this as this is a great improvement anyway, and I will create a separate issue to describe improvements left. It'll be easier to iterate from this.

@shcheklein shcheklein merged commit b291fce into iterative:master May 21, 2020
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.

doc engine: fix scroll bar for the content sidebar
3 participants