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

The sidebar in the external reviews page doesn't use the full window height #3852

Closed
past opened this issue May 6, 2024 · 2 comments · Fixed by #3864
Closed

The sidebar in the external reviews page doesn't use the full window height #3852

past opened this issue May 6, 2024 · 2 comments · Fixed by #3864
Labels

Comments

@past
Copy link
Collaborator

past commented May 6, 2024

The sidebar in the new external reviews page looks like this with a tall window:

Screenshot 2024-05-06 3 43 26 PM

Visiting one of the subpages with enough content shows the full sidebar. Conversely the spec mentors page uses the full height:

Screenshot 2024-05-06 3 44 20 PM

CC @jyasskin .

@past past added the bug label May 6, 2024
@jyasskin
Copy link
Collaborator

jyasskin commented May 6, 2024

This is true for all short pages, e.g. https://chromestatus.com/myfeatures/starred.

#content {
margin: 0;
position: relative;
min-height: 500px;
}
seems to have the result of giving the sidebar at least 500px regardless of the main page content, but it should really have a height of 100%... Changing that div and this one to have a height: 100% property appears to fix the problem.

@KyleJu, you touched this last; do you think that's the right fix, or am I missing a better way to do it?

@KyleJu
Copy link
Collaborator

KyleJu commented May 7, 2024

This is true for all short pages, e.g. https://chromestatus.com/myfeatures/starred.

#content {
margin: 0;
position: relative;
min-height: 500px;
}

seems to have the result of giving the sidebar at least 500px regardless of the main page content, but it should really have a height of 100%... Changing that div and this one to have a height: 100% property appears to fix the problem.
@KyleJu, you touched this last; do you think that's the right fix, or am I missing a better way to do it?

I think that will fix the issue. 500px might come from the minimum height from the content container, that shows the feature list. It is an issue when the list is empty.

Thanks for the fix!

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

Successfully merging a pull request may close this issue.

3 participants