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 tag filters to showcase page #4515

Merged
merged 19 commits into from
Apr 22, 2021

Conversation

lisa761
Copy link
Contributor

@lisa761 lisa761 commented Mar 25, 2021

Motivation

closes #4238

Have you read the Contributing Guidelines on pull requests?

Yes

Additional Information

Right now, I am trying to update the sites rendered by using
filteredUsers = users.filter((user) => user.tags.includes(selectedTags)).
But this does not seem to work as includes is only defined on arrays, and although user.tags is an array in user.js it appears to be treated as an object inside the filter function.

There are two ways to go about this:

  • either write a custom function for includes
  • or convert the object into an array as @javidrashid suggested

Let me know if any of these work or if there is another better way to go about this.

@facebook-github-bot facebook-github-bot added the CLA Signed Signed Facebook CLA label Mar 25, 2021
@netlify
Copy link

netlify bot commented Mar 25, 2021

[V1]

Built with commit 7c6c060

https://deploy-preview-4515--docusaurus-1.netlify.app

@netlify
Copy link

netlify bot commented Mar 25, 2021

[V2]

Built with commit 7c6c060

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

@github-actions
Copy link

github-actions bot commented Mar 25, 2021

⚡️ Lighthouse report for the changes in this PR:

Category Score
🟠 Performance 69
🟢 Accessibility 96
🟢 Best practices 100
🟢 SEO 100
🟢 PWA 95

Lighthouse ran on https://deploy-preview-4515--docusaurus-2.netlify.app/

Copy link
Collaborator

@slorber slorber left a comment

Choose a reason for hiding this comment

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

thanks

that's the idea but you should use React state and embrace the React way of doing things ;)

website/src/pages/showcase/index.js Outdated Show resolved Hide resolved
website/src/pages/showcase/index.js Outdated Show resolved Hide resolved
@slorber slorber changed the title docs(v2): add filter fto showcase docs(v2): add filters to showcase page Mar 25, 2021
@slorber slorber changed the title docs(v2): add filters to showcase page docs(v2): add tag filters to showcase page Mar 25, 2021
@slorber slorber added the pr: documentation This PR works on the website or other text documents in the repo. label Mar 25, 2021
@lisa761 lisa761 marked this pull request as ready for review April 8, 2021 07:27
@lisa761 lisa761 requested a review from lex111 as a code owner April 8, 2021 07:27
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.

We definitely don't need to show tags with empty sites, for example:

image

And ideally I'd rather see human readable labels something like that:

image

@slorber
Copy link
Collaborator

slorber commented Apr 21, 2021

Hi @lisa761

I'm completing this PR and going to merge it tomorrow, please don't add new commits

@slorber slorber merged commit 458af07 into facebook:master Apr 22, 2021
@slorber slorber changed the title docs(v2): add tag filters to showcase page feat(v2): add tag filters to showcase page Apr 22, 2021
@slorber slorber added pr: new feature This PR adds a new API or behavior. and removed pr: documentation This PR works on the website or other text documents in the repo. labels Apr 22, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed Signed Facebook CLA pr: new feature This PR adds a new API or behavior.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Improved Showcase page: tags + search engine + migrate v1 users
5 participants