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

Spacing and style in search bar #312

Closed
fcoveram opened this issue Oct 31, 2023 · 8 comments · Fixed by #340
Closed

Spacing and style in search bar #312

fcoveram opened this issue Oct 31, 2023 · 8 comments · Fixed by #340
Assignees
Labels
Milestone

Comments

@fcoveram
Copy link

As described here, the search bar has new changes:

Style

  • In Article post-type, the search bar always have a border bottom of 1px in Light Grey
  • When search bar is below the readcrumb, padding-top is 10px. When is not, padding-top is 20px.

New design of search bar area in Article


Design

Developer mockups. Updated with these changes.

@adamwoodnz adamwoodnz added this to the MVP milestone Nov 1, 2023
@adamwoodnz
Copy link
Contributor

@fcoveram so is the search bar sticky, and is it only on this page?

Currently all parts of the header are sticky, so this addition will result in a large chunk of the page being taken up. Have we decided if that's to change?

@fcoveram
Copy link
Author

fcoveram commented Nov 2, 2023

the search bar sticky, and is it only on this page?

No sticky search bar. The only sticky component in this page is the table of content.

I suggested here to only keep the local nav fixed, as in the example shared by @ryelle. But not sure whether that's being addressed or not.

@adamwoodnz
Copy link
Contributor

No sticky search bar. The only sticky component in this page is the table of content.

Ah ok. This design seems to contradict that, it shows header, chapter list and ToC sticky, unless I'm mistaken.

I suggested here to only keep the local nav fixed, as in the example shared by @ryelle. But not sure whether that's being addressed or not.

Yeah I think maybe we need to get the header changes shipped before we can move forward with Developer and Documentation.

@jasmussen
Copy link
Collaborator

jasmussen commented Nov 3, 2023

Ah, thanks for clarifying. Yes this was a mistake in the mockup showing "scrolled", I forgot the header.

Here's what it would actually look like, with the sticky behavior as proposed, correct me if I'm wrong @fcoveram:

Screenshot 2023-11-03 at 10 10 42

@fcoveram
Copy link
Author

fcoveram commented Nov 3, 2023

The version shown above is correct. Local nav fixed, and global header + breadcrumb non fixed.

@adamwoodnz
Copy link
Contributor

adamwoodnz commented Nov 5, 2023

Thanks for clarifying.

Local nav fixed

Plus ToC, and chapters (looking at image above)?

Currently chapters are not, and we'd need to add the logic the ToC has for looking at page height and determining whether it can fit and be fixed, or needs to scroll because it's longer than the window. It'll by slightly more complex because it can be expanded.

@jasmussen
Copy link
Collaborator

jasmussen commented Nov 6, 2023

Having the chapters and table of contents be sticky is an entirely not-urgent optimization we can do at any time, even after relaunch, and as with other similar elements would be the same across the site.

For what it's worth, Chapters could scroll independently. That is, be always 100% height and have overflow-y: auto;. It could be combined with a scrollbar like we use in the site editor.

Here's a mixin we use:
@mixin custom-scrollbars-on-hover($handle-color, $handle-color-hover) {

	// WebKit
	&::-webkit-scrollbar {
		width: 12px;
		height: 12px;
	}
	&::-webkit-scrollbar-track {
		background-color: transparent;
	}
	&::-webkit-scrollbar-thumb {
		background-color: $handle-color;
		border-radius: 8px;
		border: 3px solid transparent;
		background-clip: padding-box;
	}
	&:hover::-webkit-scrollbar-thumb, // This needs specificity.
	&:focus::-webkit-scrollbar-thumb,
	&:focus-within::-webkit-scrollbar-thumb {
		background-color: $handle-color-hover;
	}

	// Firefox 109+ and Chrome 111+
	scrollbar-width: thin;
	scrollbar-gutter: stable both-edges;
	scrollbar-color: $handle-color transparent; // Syntax, "dark", "light", or "#handle-color #track-color"

	&:hover,
	&:focus,
	&:focus-within {
		scrollbar-color: $handle-color-hover transparent;
	}

	// Needed to fix a Safari rendering issue.
	will-change: transform;

	// Always show scrollbar on Mobile devices.
	@media (hover: none) {
		& {
			scrollbar-color: $handle-color-hover transparent;
		}
	}
}

It should look kinda like this.

Let me know if you'd prefer I open a separate issue for this.

@adamwoodnz
Copy link
Contributor

Thanks, that helps. I think we can get the ToC updated, it's pretty close.

Let me know if you'd prefer I open a separate issue for this.

A new issue covering the chapter list sticky behaviour would be great please! We already have issues for collapsing it on mobile, and the show/hide behaviour.

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