-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Update search page style #1286
Update search page style #1286
Conversation
- Fix having multiple vertical scrollbars - Update icons lazy loading accordingly
This looks excellent! Thank you very much! |
WHAT!!? CommitDate: Mon Jun 5 22:42:31 2023 +0300
Update search page style
- Fix having multiple vertical scrollbars
- Update icons lazy loading accordingly
---
_includes/css/nerd-font-tweaks.scss | 4 ++--
cheat-sheet.js | 9 ++++++---
2 files changed, 8 insertions(+), 5 deletions(-) This looks so trivial, I have expected some very complicated stuff :-D 👍 |
What do you think about the commit I added, that...
...starts the page up like this |
[why] The additional hints in the bottom are hard to notice if we flood users with all available items first. Usually users want to search the icons and not browse, so showing them all per default seems better. [how] Just do not change the search term from "" to something else, so the search comes up empty on an empty search. If people search for a blank the previous mechanics to show all icons is triggered instead. A message to hint for this is added to the empty search result. Signed-off-by: Fini Jastrow <ulf.fini.jastrow@desy.de>
Fixed missing semicoli, force push (my js is a bit rusted). |
While this is still open, for further UX improvement I'd suggest adding autofocus on the search box when the page gets loaded (just like google.com does) |
Another suggestion: the text "Enter (part) of a word to search" is not grammatically correct usage of parantheses. The correct version would be this: "Enter (a part of) a word to search" |
I have bad experience with this autofocus stuff, but that might be a browser problem, where reloading a page steals my focus from other applications or something. But, well, why not. If you could supply an idea what needs to change I can do that together with the grammar correction (tnx, that is the result of doing multiple things in parallel I guess 😬 ). |
It's a good point, didn't think of that. I'm busy this week but I can add that next week. Maybe in another PR so we don't block this one if things took time. @Finii, the whole website requires a new modern look but then we need someone to help with UI/design. Let me know if I can help with that |
That works for showing the comments below, but I think if the website redesigned so the upper section is smaller then we can have those comments at the top, with smaller font and maybe a catchy color or effect |
So I just merge this now, and more changes can come later. If @rszyma just comments with a diff or idea regarding the focus I can just add that without PR. |
Description
Fix having multiple vertical scrollbars while searching the icons, it's easier to use and better for User Experience. It's also update the lazy loading to work on the document scrolling instead of icons container
Requirements / Checklist
What does this Pull Request (PR) do?
How should this be manually tested?
Any background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)