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

Interactive docs - make bottom sidebar items fixed to bottom (refs #5370) #5516

Merged
merged 1 commit into from
Oct 23, 2017
Merged

Interactive docs - make bottom sidebar items fixed to bottom (refs #5370) #5516

merged 1 commit into from
Oct 23, 2017

Conversation

kwertyops
Copy link
Contributor

@kwertyops kwertyops commented Oct 19, 2017

Description

Closes #5370

UPDATE:

Current solution is to make .menu-list-bottom: {position: fixed; width: inherit;}, and .menu-list: {width: inherit;}

This appears to have the desired effect of keeping the Auth/Code buttons always at the bottom of the sidebar, and always above the rest of the menu items.

The below info is no longer accurate, the solution in this branch is no longer using "position: sticky". I'm leaving the below info for historical purposes.

Simply changing the CSS position of menu-list-bottom to 'sticky' instead of 'absolute' causes the Authentication/Source Code buttons to operate as expected.

NOTE: The 'position: sticky' is a more recent CSS feature, and so browser support is not universal. See here for a list of supported browsers: http://caniuse.com/#feat=css-sticky. Notably, it appears that this change would not work in any version of IE.

Old Behavior

The Authentication and Source Code language selection buttons on the bottom left of the interactive API docs overlap with endpoints in the navigation bar on the left when there are enough endpoints. When you uncollapse an endpoint close to these buttons it opens under these buttons. (see linked issue above)

New Behavior

The Authentication and Source Code selection buttons consistently appear visible below all endpoints, while the endpoint list remains scrollable. Interaction with the buttons (collapsing and un-collapsing) also works as expected - the full "bottom menu" is always visible and interactable at the bottom of the sidebar.

@carltongibson
Copy link
Collaborator

@andrewhannum if we duplicate the position declaration (first time for IE with the existing absolute value) does IE do the right thing and just ignore the second sticky declaration?

If so that would give us a fallback here

@kwertyops
Copy link
Contributor Author

@carltongibson Yes that is correct, we should be able to do fallback like that (see here).

Though if we are going to make a fallback I think it might make more sense for the fallback to be something that isn't as broken as we know "position: absolute" is.

Maybe the fallback could be "position: static", as suggested by @JonasVDB-awingu in #5370

That would mean that in browsers too old to support sticky, the Auth/Code menu items would always appear at the very bottom of the sidebar (requiring scrolling all the way down to see). That doesn't seem too unreasonable for functionality in such old browsers. At least it wouldn't appear as glitchy as "position: absolute" currently does.

It's worth noting that we are already using "position: static" as the default style for screen widths less than 767 px. (https://github.com/encode/django-rest-framework/blob/master/rest_framework/static/rest_framework/docs/css/base.css#L263)

So that is apparently already the accepted solution for a certain edge case. (ie: if you are on mobile, and you drop-down the menu, then you have to scroll all the way down to see the Auth/Code buttons).

What do you think of using "position: static" as the fallback?

@kwertyops
Copy link
Contributor Author

(Also worth noting that "static" is the default value for position, so if we are okay with falling back on static, then it wouldn't need to be explicitly declared. That is to say, this branch as it currently stands will already fall back to "position: static" in IE :p)

@carltongibson
Copy link
Collaborator

@andrewhannum Hmmm.

My quick experiments with Safari are not having the desired results here. Safari requires a prefixed version -webkit-sticky, and the positioning is not correct with that.

As such, I'm not yet convinced this approach will work. I need to test across browsers. Screenshots here would help.

Also, I since this is reported for a "small enough window" I'm wondering if the absolute fallback would be OK — for "normal" sizes having the buttons at the bottom is the intended effect.

@kwertyops
Copy link
Contributor Author

I think there might be some confusion about the expected behavior here. This change will not always keep the buttons at the very bottom of the sidebar - it will always keep them at the bottom of the menu items, BUT when they reach the bottom of the sidebar they will stay there, and not go past the bottom, thus will always be visible above the rest of the menu items.

Let me include some examples images:

screen shot 2017-10-20 at 9 17 24 am

Above is "position: absolute". The buttons are at the bottom even though they are not being "pushed" down there. The bottom is their "home", but scrolling when the list is large causes the known problems, as seen here:

screen shot 2017-10-20 at 9 20 40 am

--

Next is "position: static" - this will keep the buttons always at the bottom of the list, though not necessarily the bottom of the sidebar. When menu items are collapsed it looks like this:

screen shot 2017-10-20 at 9 16 00 am

When menu items are expanded, the buttons stay at the bottom of the list, even being pushed off of the visible window. You have to scroll down to see them if the menu is larger than the screen. It looks like this:

screen shot 2017-10-20 at 9 24 56 am

--

Finally with "position: sticky", the collapsed state looks essentially identical to "static", where the buttons are at the end of the list:

screen shot 2017-10-20 at 9 16 00 am

But when the menu items are expanded past the bottom of the page, then the buttons stay stuck at the bottom, on top of other elements:

screen shot 2017-10-20 at 9 16 29 am

Note that all of the above screenshots were done in Safari, using the -webkit-sticky version (which you are correct, would ultimately be necessary). So even in Safari, sticky is working as I would expect, but maybe this is not how you are expecting it to work?

Possibly you are looking for the collapsed state to operate like "position: absolute" (buttons at the bottom of the sidebar, rather than the bottom of the menu items), and then the expanded state to act like "sticky" (buttons stay at the bottom, above the other items).

If that is what you want, then it appears that this is an incomplete solution.

To summarize - this branch, with the addition of the extra prefixes, will operate in a way that is more consistent and more usable than the current implementation, but not necessarily "ideal" depending on your preferences. I would be happy with this functionality, and given that @JonasVDB-awingu settled on just using "position: static", I would expect that they would be happy as well - since this seems to be an improvement on that.

If you're attached to the idea of the buttons appearing at the bottom even when they don't need to (ie. when there is room after the route menu items) then I can keep tinkering around, but it's looking like that would require, as you were guessing, some kind of flex positioning changes in multiple locations in the CSS.

@carltongibson
Copy link
Collaborator

Yes, I was expecting the "plenty of space" scenario to be unchanged.

@kwertyops
Copy link
Contributor Author

Okay actually give me one minute I think I have a better solution that is browser-agnostic and operates as you would expect.

@kwertyops
Copy link
Contributor Author

@carltongibson Please try with the current state of the branch. I've moved away from position: sticky. The new solution is:

.menu-list-bottom = {position: fixed; width: inherit;},
and
.menu-list: {width: inherit;}

I've updated the PR description. This appears to be working as expected, in all browsers!

@carltongibson
Copy link
Collaborator

Awesome. Thanks. I’ll review fully on Monday now.

Goal here is the best we can do, so your effort is appreciated!

@kwertyops kwertyops changed the title Interactive docs - make bottom sidebar items sticky (refs #5370) Interactive docs - make bottom sidebar items fixed to bottom (refs #5370) Oct 20, 2017
Copy link
Collaborator

@carltongibson carltongibson left a comment

Choose a reason for hiding this comment

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

Brilliant. position: fixed works great. (Especially given the small footprint.)

We'll take this. If users need further customisation we'll recommend overriding the templates.

@andrewhannum Thanks for your input!

@carltongibson carltongibson added this to the 3.7.2 milestone Oct 23, 2017
@carltongibson carltongibson merged commit 916a4a2 into encode:master Oct 23, 2017
@kwertyops kwertyops deleted the bottom-sidebar-sticky branch October 23, 2017 15:12
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.

Authentication and source code selection buttons in interactive API docs overlap with API endpoints
2 participants