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: make pagination-nav arrows stylizable #196

Merged
merged 1 commit into from
Dec 16, 2021
Merged

feat: make pagination-nav arrows stylizable #196

merged 1 commit into from
Dec 16, 2021

Conversation

noomorph
Copy link
Contributor

This issue stems from: facebook/docusaurus#6053

Short summary:

I'd like to stylize the pagination-nav so it looks like this:

image

The problem is that the arrows » are just inlined into the markup and cannot be hidden via CSS:

image

This is why I move them to ::before and ::after pseudo-elements, so that later on I am able to hide them completely and continue my styling described above.

@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Dec 13, 2021
@netlify
Copy link

netlify bot commented Dec 13, 2021

✔️ Deploy Preview for infima ready!
Built without sensitive environment variables

🔨 Explore the source changes: d124f0c

🔍 Inspect the deploy log: https://app.netlify.com/sites/infima/deploys/61b733e824d96800083542af

😎 Browse the preview: https://deploy-preview-196--infima.netlify.app

content: '« '
}

&__item--next &__label::after {
Copy link
Contributor

Choose a reason for hiding this comment

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

Same as above -- put it inside into the already existing rule on 25 line.

packages/core/styles/components/pagination-nav.pcss Outdated Show resolved Hide resolved
@noomorph
Copy link
Contributor Author

noomorph commented Dec 13, 2021

@lex111 I followed the suggestion with ^ selector, thanks, but maybe I'll disagree with the suggested relocation itself. Could you check out my PR once again? Doesn't it make sense to put ::before and ::after under the scope of the parent element (&__label)?

@noomorph noomorph requested a review from lex111 December 13, 2021 14:17
Copy link
Contributor

@lex111 lex111 left a comment

Choose a reason for hiding this comment

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

It's all right, thanks!

@slorber
Copy link
Collaborator

slorber commented Dec 15, 2021

Hey

I'm not against this change, but IMHO it won't solve your use-case

At most you would be able to do:

image

And not:

image

(which should rather be handled with swizzle IMHO)

@lex111 feel free to merge if you consider it's a good improvement

@noomorph
Copy link
Contributor Author

noomorph commented Dec 15, 2021

@slorber well, I'm just disabling those ::after, ::before and adding two new ones (with a background-image) to the parent container.

@lex111
Copy link
Contributor

lex111 commented Dec 16, 2021

Yep, it's still a good improvement since it allows to control the arrow in pagination links.

@lex111 lex111 merged commit c3799e1 into facebookincubator:main Dec 16, 2021
@noomorph noomorph deleted the feat/pagination-nav-arrows branch December 17, 2021 13:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants