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

Flatten the rendered page structure? #1668

Open
Fil opened this issue Sep 17, 2024 · 9 comments · May be fixed by #1669
Open

Flatten the rendered page structure? #1668

Fil opened this issue Sep 17, 2024 · 9 comments · May be fixed by #1669
Labels
enhancement New feature or request

Comments

@Fil
Copy link
Contributor

Fil commented Sep 17, 2024

The page structure currently is the following:

  • html
    • head
      • observable scripts
      • custom head
    • body
      • sidebar (optional)
      • toc (optional)
      • center
        • header (custom)
        • main (page content)
        • footer (custom)

there are lots of optional and custom content, but the fact that some of it is nested in the center div makes it a bit difficult to reorganize with css (flexbox or grid).

It would feel nice to flatten the body:

  • body
    • sidebar (optional)
    • toc (optional)
    • header (custom)
    • main (page content)
    • footer (custom)

I experimented a bit, and it doesn't seem to require many changes in grid.css — basically we just have to tack an .observablehq-center class on the header, main and footer elements, and change a few of the margins.

An alternative (or complement) would be to reboot #253 and allow "full page templates".

@Fil Fil added the enhancement New feature or request label Sep 17, 2024
@CobusT
Copy link
Contributor

CobusT commented Sep 17, 2024

This change would give users a lot of flexibility with the page layout.

@Fil Fil linked a pull request Sep 17, 2024 that will close this issue
1 task
@mbostock
Copy link
Member

I expect it will be difficult to do this in a backwards-compatible way, and it will probably cause a lot of churn with the CSS. I recommend we consider more incremental solutions.

@Fil
Copy link
Contributor Author

Fil commented Sep 18, 2024

#1669 shows how much churn there is (not much, I would say).

If we absolutely must keep the same nested page structure for backwards compatibility, then I think we'll have to revive something like #253.

With the current code base, if you want to have a header that takes 100% of the width (with a sidebar that starts below it), you can technically do so by inserting it in head, but it's clearly wrong since it's supposed to fill in the <head>.

@mbostock
Copy link
Member

Okay. I can take a look at #1669 when I have time but it’s not yet done and I see some hints of difficulties (do the container media queries still work? how does this affect custom styles in userland?).

@CobusT
Copy link
Contributor

CobusT commented Sep 18, 2024

I am pretty sure the container media queries still work. I went through them yesterday and tested all the breakpoints.

@mbostock
Copy link
Member

Since I haven’t had a chance to test yet, perhaps someone would be kind enough to explain how the container queries still work given that container-type: inline-size; is removed? The way I typically test this is to choose a relatively narrow window width and then toggle the sidebar; this changes the width of the container, and should toggle the visibility of the table of contents and the number of available columns in grid layouts.

@CobusT
Copy link
Contributor

CobusT commented Sep 18, 2024

Ah! Those container queries. They will probably break. I will investigate and report back.

@CobusT
Copy link
Contributor

CobusT commented Sep 18, 2024

I added the container-type: inline-size; back (on the #observablehq-main div (instead of the #observablehq-center div that is no more. That fixes the problem with the grid layouts. However, it pushes the #observablehq-main div a little bit down on the page and I don't understand why.

@mbostock
Copy link
Member

It disables margin collapse.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants