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

feat(V2): add custom wrapper class to SearchPage, fix title #3834

Merged
merged 3 commits into from
Nov 27, 2020

Conversation

Simek
Copy link
Contributor

@Simek Simek commented Nov 27, 2020

Motivation

Currently there is not possible to customize the SearchPage styling without swizzling the component.

This PR adds the custom wrapper class name to the SearchPage, so it will be possible to add custom styles which only apply to this page.

Have you read the Contributing Guidelines on pull requests?

Yes.

Test Plan

Local run of Docusaurus V2 website.

Preview

Screenshot 2020-11-27 204101

Related PRs

No.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Nov 27, 2020
@netlify
Copy link

netlify bot commented Nov 27, 2020

Deploy preview for docusaurus-2 ready!

Built without sensitive environment variables with commit 2e73641

https://deploy-preview-3834--docusaurus-2.netlify.app

@slorber
Copy link
Collaborator

slorber commented Nov 27, 2020

Hi,

PR should pass after updating your branch, it's an error on master I fixed

@Simek Simek force-pushed the add-search-page-wrapper-class branch from 8750f4a to 2246073 Compare November 27, 2020 20:29
@@ -294,7 +294,7 @@ function Search() {
}, [searchValue]);

return (
<Layout title={getTitle()}>
<Layout title={getTitle()} wrapperClassName="search-results-wrapper">
Copy link
Contributor

Choose a reason for hiding this comment

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

search-page-wrapper is better suited because "results" can only refer to a specific section of this page.

@Simek
Copy link
Contributor Author

Simek commented Nov 27, 2020

I have decided also to add small fix for the title of the SearchPage.

Currently the page title do not follow the general pattern which includes titleDelimiter and siteTitle. Also passing the title to Layout instead of placing it into the Head produced warning:

Missing required 'title' element

Preview

Screenshot 2020-11-27 214832

@Simek Simek changed the title feat(V2): add custom wrapper class to SearchPage feat(V2): add custom wrapper class to SearchPage, fix title Nov 27, 2020
@lex111 lex111 merged commit bb387e0 into facebook:master Nov 27, 2020
@lex111
Copy link
Contributor

lex111 commented Nov 27, 2020

@Simek thanks!

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

Successfully merging this pull request may close these issues.

None yet

4 participants