Flyout: define `position="inline"` #500

humitos · 2025-01-16T14:32:13Z

Allow theme developers to define position="inline" to adapt the flyout to its container. It uses bottom: 0 to make it open up side, and it should probably use top: 0 to make it open down side.

Allow theme developers to define `position="inline"` to adapt the flyout to its container. It uses `bottom: 0` to make it open up side, and it should probably use `top: 0` to make it open down side.

ericholscher

Simple enough!

src/flyout.js

src/flyout.css

agjohnson · 2025-01-16T19:25:33Z

Here is a very rough example of inline positioning: #501

It doesn't take much more but there is some cleanup required and working into themes might require experimenting with overflow clipping on the theme elements.

Here is a quick example of using inline positioning. I've reused `top-left` position to indicate "downward expanding menu" but this could be something more explicit. It would be nice as `position="inline down"` perhaps. There are two basic examples, one truly inline and one flyout inside a mock sidebar menu -- just to illustrate a theme maintainer nesting the flyout element inside another element. The tricky part here is making the `main` menu "float" over other content, and we use `overflow: visible` and absolute positioning to allow content to reflow around the element. There are some fixes required here: too narrow of a parent clips the collapsed menu, we probably want a max width on the popup menu (keeping the collapsed menu width 100% (this is to fill the parent element). https://github.com/user-attachments/assets/413d7c03-38d3-48e0-a88a-4cf53cc8dd92

humitos · 2025-01-20T13:25:18Z

This is an example of what I currently have. It shows the current default floating bottom right position, then the bottom inline position and finally the top inline position.

I think we should probably change bottom and top by down and up classes, meaning what's the direction the flyout opens.

humitos · 2025-01-20T14:20:37Z

Read the Docs theme

humitos · 2025-01-20T15:37:14Z

Sphinx with Furo theme

Furo fork that injects the flyout: https://github.com/pradyunsg/furo/compare/main...humitos:furo:readthedocs-flyout-inline?expand=1

ericholscher · 2025-01-20T15:44:56Z

The furo theme isn't showing properly in the sidebar, right?

humitos · 2025-01-20T16:05:41Z

It works for me. I uploaded a GIF in my previous comment. Why are you asking?

ericholscher · 2025-01-20T18:39:19Z

It's overlapping? The red line is the sidebar divider, right?

humitos · 2025-01-21T09:17:33Z

Ah, yes. I think that is responsibility of the theme author, not the flyout itself -- but I'm not 100% sure. I understand the flyout container has to be adjusted with CSS properties to make the flyout behave as expected.

humitos · 2025-01-21T09:34:59Z

By the way, removing the padding: 0 10px from .container.inline makes the flyout to match the sidebar size on Furo. However, it doesn't look as we want because the logo and the caret has no padding and touch the borders. Also, it doesn't cover all the width of the sidebar on Read the Docs theme.

humitos · 2025-01-21T09:43:50Z

I was able to fix the in Furo adding CSS in the flyout container without changing anything in our code. So, this is responsibility of the theme developer.

Branch on forked Furo repository: https://github.com/pradyunsg/furo/compare/main...humitos:furo:readthedocs-flyout-inline?expand=1

humitos · 2025-01-21T11:04:16Z

I tested this implementation on insipid theme as well.

Note that it works fine, it just requires adding some specific style to the flyout container already defined in the theme. These are the override changes used in test-builds: readthedocs/test-builds@insipid-theme?expand=1#diff-d3eab869c9

Note there is an extra pixel in the left side. I'm trying to fix that. I need to find the correct formula for .container.inline main { margin-left: calc(var(--addons-flyout-font-size) - 1.3rem )} cc @agjohnson

humitos · 2025-01-21T11:37:16Z

Boom! I was able to find the CSS combination that works 👍🏼

Example for Insipid, Read the Docs and Furo themes.

humitos · 2025-01-21T12:25:10Z

I used JavaScript to calculate the main.margin-bottom's property based on the header.height's property because I think there is no way in CSS to use a value based on another element's property. This code allowed me to avoid issue with extra pixels when clicking on the flyout due to the height of the header -- which could vary depending on the theme.

agjohnson · 2025-01-21T17:34:02Z

src/flyout.js

+        );
+      }
+    }
+  }


So we should always avoid doing anything with JS, especially with Element.style. What this property does is add inline style="" attribute to the element, which causes trouble with secure CSP rulesets and causes problems when users try to override our styles.

What value do we not know that we need to perform calculations in JS to get?

I know, but I did it in JS to show this is possible at least.

I wrote what I need to do in #500 (comment)

Do you know how to do that with pure CSS? 🤔

In CSS this seems like it should just be addition. If we're mixing units, we should change them all to em so that the math is just a simple line-height + margin-bottom + margin-top + padding-bottom + padding-top. The header height should be static, we set the font size and don't allow wrapping, so there shouldn't be guessing there either.

What value do we not know that we need to perform calculations in JS to get?

This isn't clear to me still, so maybe I'm missing something here.

What value do we not know that we need to perform calculations in JS to get?

I need to calculate the main.margin-bottom's property based on the header.height's property. How I can do that in CSS?

header.height is not static since it will be based on the container where the theme author inject the <readthedocs-flyout> element. How I can get that value with pure CSS?

We don't need to calculate it because we should know this value.

header.height is not static since it will be based on the container where the theme author inject the element.

This doesn't seem correct. If the parent element is 10em, we get this effect if we try to fill vertical space with 100% height:

We instead want the header to be static height, and derived from font-size. We should be able to determine the value needed for margin-bottom just by math based on this.

Ah, I thought we wanted height and width to be dynamic and based on its container. If we want a fixed height, that should be easier to calculate, yeah.

OK. I added .container.closed { height: 2.5rem } and kept the .container { height: auto } so it grows accordingly when it's clicked.

humitos · 2025-01-23T10:32:37Z

I dug a little more here and I think the different behavior between our theme and furo theme is that furo uses flex for the navigation bar and we don't. This is just a guess because the they it expands and fulfill the spacing.

Note that removing the hack JS code and extra CSS, the flyout grows properly on furo. The header goes to the top as it should be, instead of keeping below the main content as I showed in the other examples.

However, with this code, the flyout in our theme grows to the bottom when clicked and its not shown in the page. I tried playing with flex in our theme, but I wasn't able to get it working 😢

humitos · 2025-01-23T10:48:38Z

Yeap, using display: flex and flex-direction: column on the nav of our theme made the trick! It expands properly now --leaving the header of the flyout above the main content of it.

agjohnson · 2025-01-27T21:07:20Z

Seems you lost the up/down distinction, this should open up:

There is some positioning issues on the inline flyout inner element too it seems:

humitos · 2025-01-28T12:03:34Z

When opening "up", how do we want the flyout to behave?

Move the header to the top	Keep the header at the bottom

If we want "Move the header to the top", I wasn't able to figure it out how to do that when .inline.

Flyout: define position="inline"

186fde2

Allow theme developers to define `position="inline"` to adapt the flyout to its container. It uses `bottom: 0` to make it open up side, and it should probably use `top: 0` to make it open down side.

humitos requested a review from a team as a code owner January 16, 2025 14:32

humitos requested a review from agjohnson January 16, 2025 14:32

Add some padding to the flyout respecting the container

9c8806d

This was referenced Jan 16, 2025

Add readthedocs-flyout explicitly using position=inline readthedocs/sphinx_rtd_theme#1637

Open

Use explicit readthedocs-flyout with inline position readthedocs/test-builds#2118

Closed

Update tests to pass

c242b50

ericholscher approved these changes Jan 16, 2025

View reviewed changes

src/flyout.js Show resolved Hide resolved

agjohnson reviewed Jan 16, 2025

View reviewed changes

src/flyout.css Outdated Show resolved Hide resolved

src/flyout.css Outdated Show resolved Hide resolved

humitos mentioned this pull request Jan 20, 2025

Flyout: overlapping content #395

Open

agjohnson and others added 2 commits January 20, 2025 14:19

I added my changes

f95fae5

humitos added 2 commits January 20, 2025 14:32

Rename CSS classes

55ab856

Increase the size of the container and grow with it

60b9e33

humitos requested a review from agjohnson January 20, 2025 13:33

humitos added 2 commits January 20, 2025 15:47

Fix down/up openings

3372883

adjustments for Sphinx Furo theme

fa162e5

humitos mentioned this pull request Jan 21, 2025

Please provide a way for Sphinx themes to "take over" the flyout menu readthedocs/readthedocs.org#11688

Open

Caculated properties are not needed

721cfa7

Calculated width is not neeed here

bf6962c

Use JavaScript to calculate margin-bottom based on main's height

4af9427

humitos added 2 commits January 21, 2025 13:30

Same for .down class

49e6b4e

Keep width because is required in some scenarios

8acadc3

agjohnson reviewed Jan 21, 2025

View reviewed changes

Remove JS hack code

4e40ec2

humitos requested a review from agjohnson January 23, 2025 10:48

humitos mentioned this pull request Jan 28, 2025

Docs: explains flyout position readthedocs/readthedocs.org#11954

Merged

More CSS tricks

bfde1ec

ferdnyc mentioned this pull request Jan 28, 2025

page.html: Position the flyout to bottom-left, on ReadTheDocs pradyunsg/furo#865

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Flyout: define `position="inline"` #500

Flyout: define `position="inline"` #500

humitos commented Jan 16, 2025 •

edited

Loading

ericholscher left a comment

agjohnson commented Jan 16, 2025

humitos commented Jan 20, 2025

humitos commented Jan 20, 2025

humitos commented Jan 20, 2025

ericholscher commented Jan 20, 2025

humitos commented Jan 20, 2025

ericholscher commented Jan 20, 2025 •

edited

Loading

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025 •

edited

Loading

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025

agjohnson Jan 21, 2025

humitos Jan 21, 2025

humitos Jan 22, 2025

agjohnson Jan 22, 2025 •

edited

Loading

humitos Jan 23, 2025

agjohnson Jan 27, 2025

humitos Jan 28, 2025

humitos Jan 28, 2025

humitos commented Jan 23, 2025

humitos commented Jan 23, 2025

agjohnson commented Jan 27, 2025

humitos commented Jan 28, 2025

Flyout: define position="inline" #500

Are you sure you want to change the base?

Flyout: define position="inline" #500

Conversation

humitos commented Jan 16, 2025 • edited Loading

ericholscher left a comment

Choose a reason for hiding this comment

agjohnson commented Jan 16, 2025

humitos commented Jan 20, 2025

humitos commented Jan 20, 2025

Read the Docs theme

humitos commented Jan 20, 2025

Sphinx with Furo theme

ericholscher commented Jan 20, 2025

humitos commented Jan 20, 2025

ericholscher commented Jan 20, 2025 • edited Loading

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025 • edited Loading

humitos commented Jan 21, 2025

humitos commented Jan 21, 2025

agjohnson Jan 21, 2025

Choose a reason for hiding this comment

humitos Jan 21, 2025

Choose a reason for hiding this comment

humitos Jan 22, 2025

Choose a reason for hiding this comment

agjohnson Jan 22, 2025 • edited Loading

Choose a reason for hiding this comment

humitos Jan 23, 2025

Choose a reason for hiding this comment

agjohnson Jan 27, 2025

Choose a reason for hiding this comment

humitos Jan 28, 2025

Choose a reason for hiding this comment

humitos Jan 28, 2025

Choose a reason for hiding this comment

humitos commented Jan 23, 2025

humitos commented Jan 23, 2025

agjohnson commented Jan 27, 2025

humitos commented Jan 28, 2025

Flyout: define `position="inline"` #500

Flyout: define `position="inline"` #500

humitos commented Jan 16, 2025 •

edited

Loading

ericholscher commented Jan 20, 2025 •

edited

Loading

humitos commented Jan 21, 2025 •

edited

Loading

agjohnson Jan 22, 2025 •

edited

Loading