-
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 #1265
Update search page style #1265
Conversation
I totally agree! Thanks! We need to see how this meshes with #1252. |
I noticed that when updating that but that would be better than having double scrollbars next to each other, it's confusing in terms of usability. My suggestion would be is one of these options:
I prefer the third option, in addition to removing the large search icon at the top and maybe reduce the comment to a link to releases page where that is explained. Already those removed icons indicated using the badge on the top of each icon. So, the idea is to remove one of those two scrollbars as they're kind of confusing |
At the moment I struggle to get THIS working with the current (new) cheat-sheet - apart from placement of the extra info. |
Well, but that is the unified design for all pages there. Changing this for the cheat sheet should change it throughout to keep the consistency - but that would be a major redesign and I would not touch that now. |
Fix having multiple vertical scrollbars while searching the icons, it's easier to use and better for User Experience
The cheat sheet code changed meanwhile. It loads only 250 icons and triggers to load another 250 if you scroll down (the inner thing). If we have no inner scrollbar that load-more-icons is never triggered. I can change the code such that it does NOT lazy load the icons, and of course then there is no problem. But that is kind of bad because it now per default shows all icons (over 8000) ... What I did not manage was to rewrite the new cheat-sheet code that the load-more-icons is triggered when the bottom of the page is shown/reached. Hope this makes sense. Force push to update this to the new cheat sheet code... |
Scrolled down completely, and only the first 250 icons are shown. See how 'big' the scrollbar indicator is. Edit: Search term is nothing ( |
Relevant code... Lines 26 to 32 in deaf6ab
|
I got you now, I didn't see the new change. So, you're accepting my changes but need the lazy load to work on the outer scrollbar after removing the inner one. If so, I'll work on that later today and create another PR |
I've already fixed that but need your opinion on this... What do you think is better?
Based on that I'll commit the changes and create new PR for you to review |
Ugg. Well, if there is anything below the (currently loaded) icons - and only then the question seems to have any meaning - I guess you want to load further icons one you reach the end of the icons. If you load the icons only if you reach the bottom of the page the newly loaded icons will be somehow introduced in the middle of the currently shown part of the page. That might also work, but, I guess your 2nd option is more what I would call "natural". Meaning for the user it does not really show differently with or without lazy load (if the user scrolls slow enough). I guess the difference is very subtle. Maybe only visible on very small screens (i.e. mobile).
Thank you, really appreciate |
I would move the "removed icons" and the "how to embed" stuff, that is now at the bottom, to some other place, so there is not much below the icon list anymore anyhow. |
I've created a new PR #1286 with the new changes that includes scrollbars fix and lazy loading update based on that. With regards to comments section at the bottom, I can move it but need to find a better place for that. That should also take into consideration the different screen sizes especially very small ones Can you please have a look and let me know if that works as expected? |
Closing this in favor of #1286 |
Fix having multiple vertical scrollbars while searching the icons, it's easier to use and better for User Experience
Description
Fix having multiple vertical scrollbars while searching the icons, it's easier to use and better for User Experience
Requirements / Checklist
What does this Pull Request (PR) do?
Fix scss styles
How should this be manually tested?
Same changes cap be applied using browser developer tools
Any background context you can provide?
What are the relevant tickets (if any)?
Screenshots (if appropriate or helpful)