-
Notifications
You must be signed in to change notification settings - Fork 18
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
Revise search page design #1025
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can't imagine how happy I am that the ugly old search is finally improved!
As with these design-heavy PRs, first a review purely by testing the deployment, without looking at the code. I really like it already but think it could be improved by minor changes. Here are some ideas, let me know if you agree that they look better. (For some, I'm really unsure myself).
- Maybe remove the vertical line between icon and main content?
- More padding/space between line and main content. Without the line, gap: 24px?
- A bit more space between thumbnail and main content
- Heading h3
- font-size 18px or even 17px?
- line-height 1.3?
- Icon
-
stroke-width: 1.5
? - Smaller? E.g. 32px?
- Icon choice:
- Should be very easy to distinguish at a glance. Currently both have a box around them, making them look kinda same
- Use
film
as icon for videos? - For page: file-text, scroll-text, layout-list, list-tree, scroll, gantt-chart-square, newspaper, layout-panel-top?
-
- Restrict creators to one line with "..."?
- Maybe make it so that "Part of series" is always at the bottom of the box, i.e. when there is no description?
- Maybe replace "part of series" with just "series"? And maybe add an icon in front, with the same style as the creator icon?
- Screen size stuff
- Maybe remove the left/right margin of the
<li>
completely. At small screen widths, we can use that width for content and I don't think it looks crammed. On wider screens, that margin is irrelevant anyway. - At width 680, the thumbnail takes a big part of the width, making the text look very crammed. Maybe below 800 or 750, shrink the thumbnail a bit? Bit with the previous point, it's not as bad anymore.
- At width 640, the thumbnail is pretty huge. Even if it doesn't look amazing, I would probably restrict its width to like 400px maybe (and then center align)
- And below 650, does it make sense to put the thumbnail over the title/text stuff? That would be closer to normal series blocks. Should be possible with a simple
flex-direction: column-reverse
or sth. - Mhh on very narrow screen, the "kind icon" is on the right, but taking the whole column. It is kind of unfortunate as we lose space for the description that way. Mh. But I get it, avoiding that is tricky :P Maybe we'll come up with a nice idea later.
- Maybe remove the left/right margin of the
Thanks, Ole!
+1
+1
+1
|
Thanks, Ole and Lukas for the PR and the comments. I discussed it with our interaction designer. Here Our
+1
+1
+1, but what happens, if I am looking for a creator who got replaced by the "..."?
-1, I would stick to "Part of series" so we have the same wording in the search and on the video page. I would not insert an additional icon
+1, I would suggest to reduce the size of the thumbnail in general and not to shrink it in smaller screens. I quite liked the size of the thumbnail in the 'old' search
+1, I am strongly in favor to put the thumbnail over the title/text stuff. Some additional ideas:
|
You will never ever be able to see all the possible creators. Cf. https://video.ethz.ch/events/nsl/colloquium/2023/planetary_urbanisation/2d98f6be-7847-4a46-864c-7965dfcadad5.html. So you have to restrict and one line sounds good to me. |
That's certainly useful to bring up here. I did consider it and I think it's possible to just add below the series, for example. So I think going with this design now won't bring problems down the line.
Yeah that's a good point, The same problem can occur for the description and also the title (which is restricted to two lines). We can get the information of where the match occurred (i.e. where the search term is in the description) so I suppose we can snip the part around the match then. So certainly possible, but implementing it will require some work.
I like the new larger thumbnails. YouTube has way larger ones, for comparison: I would be ok with reducing them a tiny bit still, but I think the old size is too small. Also note: if we make it smaller, this thing happens more often: the metadata/text takes more height than the thumbnail and then the thumbnail is not flush against the box borders anymore:
Very good point about the breadcrumbs/path. Those shouldn't be colored (or they should be clickable like the series title). Not super sure about not coloring the titles. I quite liked it. But lets maybe get some comparison screenshots once most other suggestions are implemented.
That is indeed planned, but not part of this PR.
Not sure I agree with that, I like the way it is. But also no strong opinion.
Yeah, I agree I guess, but it makes the problem of "thumbnail not flush" more likely to happen.
While I agree that its a bit much detail for such a small icon, we can't really do that: Tobira uses non-filled icons everywhere. Using a filled one here would be inconsistent.
Yeah... I get you. Maybe we can experiment a bit still? No idea if we can find sth nice with thumbnails on the left. I think first implementing all other things mentioned above and then reevaluating is a good idea. |
Some comments:
|
bb52a73
to
0576b12
Compare
Hm I would still prefer if it would return. But I couldn't find a good solution on the fly, so lets go with your suggestion for now. In the long run, we might want to change the search to a modal after all, that would also solve the other problem you mentioned (previous page has to be loaded and isn't there immediately). Though as we discussed, that would mean that one couldn't share search results via a link. Hmm. Lets table this for later. |
Some feedback from Olaf and David is appreciated then: should you return to the previous page when emptying the search input? Or are the methods "pressing ESC" and "clicking the X" sufficient? Any particular reason you chose One tiny thing still: the focus outline of the series title is cut off at the bottom. It being cut off on the right side if the title is cut off would be fine for me, but bottom we can fix I think. Also one thing I thought of, but this really doesn't have to be in this PR unless you can do it in 3 minutes: navigating through the results with arrow up and arrow down? I'd like that, but it's not that important I suppose. |
Yes, I tested
Yeah I'd like that as well, though I don't really see myself implementing this in 3 minutes (though tbf, there aren't a lot of things I could do in 3 minutes period). |
0576b12
to
dea16e2
Compare
frontend/src/routes/Search.tsx
Outdated
overflow: "clip", | ||
overflowClipMargin: 3, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well of course this doesn't work in everyones favorite browser. (I mean safari - which also has trouble with the focus border of the search items)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Huff yeah that looks weird. But also the focus outline on the item itself. Seems like the <li>
or sth has overflow: hidden
? That shouldn't be necessary? Alternatively, you can just ignore fancy things like clip-margin
and just add more padding? Does just using overflow-x
improve stuff? overflow-clip-margin
support indeed seems to be not great :/
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah we can just add some padding. Though you better not use shift+tab
in safari, unless you want to be sad. It seems that the focus outline weirdness of the item itself is caused by the solution that I use for the nested link. It's so annoying that safari can't handle that. Either I look for another solution, or we remove the link again. In which case it also doesn't need to be tabbable.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just for the protocol: I think this could be merged. It's a clear improvement. Some minor things (Safari focus outline, ...) can be improved later. With everything else (icon choice, ...) I can certainly live.
So I think we are just waiting for feedback from Olaf and David. In particular regarding these things:
- Coloring titles or not? (See screenshots above)
- What icon?
- Thumbnail positioning
Regarding this, I'm against moving the thumbnails to the left and the link to the TIB portal was supposed to tell you why. Which obviously wasn't self-explanatory. |
I have no strong feelings regarding this one. If you want to change your search term and use the "X" to do so, yes, you are on a different page, but your cursor is up there anyway, so you simply click once more and enter the new search term. No big deal. |
Correct. While I would like to use "play" for coherence with the player, it looks like the Tesla cybertruck of icons. Also, we could make it coherent if we make Valencia use "play-circle" in the preroll, cf. polimediaupv/paella-ethz#50. |
+1 for coloring titles. Just for the record, there is a light mode, you guys always tend to look at the dark mode, it seems. |
This comment was marked as resolved.
This comment was marked as resolved.
dea16e2
to
d695b80
Compare
Ok, let's stick with the colored titles.
+1 for the the play button with the circle around.
I would still vote for moving the thumbnail to the left. But I get it, I'm outvoted here. My main reasons to move it to the left: The most important things (thumbnail and beginning of the title) are right next to each other and can be studied in a single glance. With the thumbnail on the right they are quite far away from each other forcing the viewer to look back and forth between thumbnail and title. To sum it up: I think too, this PR can be merged as it is. |
The behavior as it is at the moment in this PR (return to the previous page when clicking the X or pressing ESC, stay at the search results when the search input was emptied) makes sense to me. |
I would agree with that. But again, putting them left has other problems. So yeah, lets keep them on the right, at least for now. With this, this seems good to go! 🎉 |
This applies some smaller changes to the appearance of search items. In particular, thumbnails are now on the right side and an icon has been added to ease differentiating between the types of these items (not super useful right now as we only have videos and pages, but other types will be added in the future).
This also adds a button to close the search/return to the previous page the search has been initiated from. This will also happen upon pressing
Esc
or clearing the search input field.Further improvements like highlighting search terms in the results and filtering for specific items will be done in a follow up PR.
Closes #883