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

docs: filter icons by URL search query #731

Merged
merged 1 commit into from
Mar 26, 2023

Conversation

korki43
Copy link
Contributor

@korki43 korki43 commented Feb 7, 2021

This fixes #192.
It's currently not possible to get the search term via list.js so I took the search term directly from the input.
The url query will update whenever the search term changes.

Example: https://deploy-preview-731--bootstrap-icons.netlify.app/?q=brand

@XhmikosR
Copy link
Member

XhmikosR commented Feb 7, 2021

Thanks for the PR!

I should spend some time to move JS outside of HTML but that's for later.

Not sure about filter or using something else like icon or q. Let's wait for @mdo's opinion.

Also, I feel like we might need a clear input button.

EDIT: @korki43 actually I have already moved JS to https://github.com/twbs/icons/blob/main/docs/assets/js/application.js, so better move that there too (might need some extra checks to prevent TypeErrors etc).

@korki43
Copy link
Contributor Author

korki43 commented Feb 7, 2021

@XhmikosR thanks for the fast response!

It seems like application.js is loaded on any page while list.js (and the code to initialize the list) is only included on the homepage.

If I include list.js in vendor and add the other code to application.js both will be loaded on any page, although they're only needed on the homepage. I also could create a new folder (e.g. homepage) in assets/js which contains the code to include only on the homepage. What would be your or @mdo's preferred way?

I also would suggest the name tags for the url search query as the search options may change over time and could include things like style in the future.

I also think it would be nice if the page automatically scrolls down to the icons if a search query is given.

@XhmikosR
Copy link
Member

XhmikosR commented Feb 7, 2021

Can't you check for a specific selector and if not present bail out?

I don't mind having a separate file, but I'd rather create a new "bundle" file with list.js and our needed code so that we can reuse it in the font page later.

Perhaps, just leave it as is for now until we sort out #706 which will reuse the same logic as the homepage.

As for scrolling, that would be a good idea.

@korki43
Copy link
Contributor Author

korki43 commented Feb 7, 2021

Of course it's possible to perform such a check, my concern was about the bundle size of application.js if we include list.js, because list.js is only needed on the home & font pages.

We could also go with a different approach and use a "share" button to copy/share the search URL instead of a constantly updating url search query.

It may be possible to use the Web Share API (or copy the link if it's not available).

To implement the "scrolling" behavior, the content anchor can be added to the link.

I also think it's better to wait for #706. I will transform this into a draft PR.

@korki43 korki43 marked this pull request as draft February 7, 2021 18:31
@korki43 korki43 closed this Feb 11, 2022
@korki43 korki43 deleted the search-query-filter branch February 11, 2022 20:41
@korki43 korki43 restored the search-query-filter branch February 11, 2022 20:42
@korki43 korki43 reopened this Feb 11, 2022
@vanillajonathan
Copy link
Contributor

@korki43 You forgot about this?

@korki43
Copy link
Contributor Author

korki43 commented Jul 13, 2022

@vanillajonathan No, as per the comment above we are still waiting for #706 to be merged.

@mdo
Copy link
Member

mdo commented Aug 28, 2022

Just merged #706! Took me long enough 😅

@XhmikosR
Copy link
Member

XhmikosR commented Sep 8, 2022

Unfortunately, we don't currently have a PR preview. I'll try to switch back to Netlify later.

EDIT: Switched repo to Netlify, we should have PR previews back from outside collaborators.

@korki43 korki43 mentioned this pull request Sep 22, 2022
7 tasks
@XhmikosR
Copy link
Member

@korki43 could you please rebase one last time?

@XhmikosR XhmikosR added docs Improvements or additions to documentation enhancement New feature or request labels Mar 24, 2023
@korki43
Copy link
Contributor Author

korki43 commented Mar 24, 2023

I'm currently unable to work on my computer, but I will rebase the branch as soon as I get back.

@XhmikosR
Copy link
Member

XhmikosR commented Mar 24, 2023

Take your time, thanks!

BTW now I guess the right place would be https://github.com/twbs/icons/blob/main/docs/assets/js/list.js

I plan to move the docs assets to modules and build them with Hugo like I do on the blog https://github.com/twbs/blog/tree/main/src/assets/js & https://github.com/twbs/blog/blob/main/src/layouts/partials/scripts.html

@twbs twbs deleted a comment from korki43 Mar 26, 2023
@korki43
Copy link
Contributor Author

korki43 commented Mar 26, 2023

Done. But I didn't know how to rebase and move the changes to another file so I just merged main into the branch and moved it manually.

Preview: https://deploy-preview-731--bootstrap-icons.netlify.app/?q=circle

@XhmikosR XhmikosR merged commit 3b27adc into twbs:main Mar 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation enhancement New feature or request
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

Filter by search query
4 participants