Add version filtering option to the lint list #8752

Serial-ATA · 2022-04-26T22:38:38Z

I'm no web dev, so I don't know if this is the best execution 😄.

Here's how it looks:

And on mobile:

I've split this into two commits, in the second one I moved the JS into its own file to make it easier to work on. Is that alright? And if so, could the same thing be done to the css?

changelog: none
cc: #7958, @repi
r? @xFrednet

flip1995 · 2022-04-28T13:09:22Z

Could we use the unicode signs for ≥, = and ≤. Also: Why not make this implicit and remove the checkboxes and just apply the filter if the text box has a valid value.

Things to consider:

Make it so that you can only specify the minor version and 1.__.0 is fixed. (I don't think specifying the patch version gives any benefits). That way you can only accept numbers in the text box and don't really have to verify any input
Drop downs für versions? Really unsure about that.

(As always with those PRs: I don't know any JS/CSS, I just want that it looks pretty 😄 )

xFrednet · 2022-04-28T13:15:44Z

Why not make this implicit and remove the checkboxes and just apply the filter if the text box has a valid value.

I really like this suggestion, some highlighting that a filter is active would still be nice IMO.

Could you add a filter counter to the dropdown button, like the other filter options already display? 🙃

Drop downs für versions? Really unsure about that.

I think it fits with the other search options. Do you have another design in mind? There were some clips with a new version box for searching, but that looked a bit off to me.

Another option could be to support direct searches in the search box, for strings "unwrap <= 1.50.0" but I doubt that may people will try that.

You should receive a full review by the end of the weekend 🙃

flip1995 · 2022-04-28T16:03:57Z

I think it fits with the other search options. Do you have another design in mind? There were some clips with a new version box for searching, but that looked a bit off to me.

Oh I think my message was ambiguous. I meant dropdowns inside the dropdown where you can select a version, so something like that:

I definitely like the top-level drop down for this filter.

Serial-ATA · 2022-04-28T19:29:26Z

I like the idea of the new symbols and implicit enabling, I'll add those in 🙂. Before I do that though, was it alright to split out the JS?

Oh I think my message was ambiguous. I meant dropdowns inside the dropdown where you can select a version, so something like that:

I'm not sure about a dropdown for versions. There'd be 30+ options (and growing) for each field.

Make it so that you can only specify the minor version and 1.__.0 is fixed. (I don't think specifying the patch version gives any benefits). That way you can only accept numbers in the text box and don't really have to verify any input

Do you mean have "1." and ".0" with an input field in the middle?

xFrednet · 2022-04-28T19:36:26Z

Before I do that though, was it alright to split out the JS?

I'll have to see how our deploy scripts works, but it should be fine from what I remember. I would be interested on what the benefit is. Doesn't this mean that it has to first load the index.html and then the files linked inside? (I'm not a frontend developer)

I know that it's common practice to do this, and I'm fine with it. For me, this is just a tool that has to work well 😂 😅

Serial-ATA · 2022-04-28T20:09:13Z

Doesn't this mean that it has to first load the index.html and then the files linked inside?

It looks like the file is fetched and executed when the script tag is reached by the parser (https://html.spec.whatwg.org/images/asyncdefer.svg). It shouldn't have any real effect, it just makes the site easier to work on.

Also, I already updated the deploy script. I'll be sure to give it a test, but it should be fine.

xFrednet · 2022-04-28T20:15:25Z

In that case it should be fine 🙃

xFrednet · 2022-04-28T21:08:32Z

BTW, since you're already working on the website, would you mind taking a look at #7959. That might be a good feature for a follow-up. The implementation was already started in #8473, which was closed due to inactivity. I would really love to have that feature. If you had the time and motivation, I would appreciate it. 🙃

flip1995 · 2022-04-28T21:13:23Z

I'm not sure about a dropdown for versions. There'd be 30+ options (and growing) for each field.

Yeah, I had the same concern. Just wanted to bring up the idea.

Do you mean have "1." and ".0" with an input field in the middle?

Yes exactly. That way, you have to type less and can't really put any garbage in the text field that the JS then would have to deal with.

Serial-ATA · 2022-04-28T21:39:14Z

BTW, since you're already working on the website, would you mind taking a look at #7959. That might be a good feature for a follow-up.

Oh yeah I'd really like to have that too, I'll try that out.

Yes exactly. That way, you have to type less and can't really put any garbage in the text field that the JS then would have to deal with.

I'll try that out, but I'm worried that'll make the input really small and difficult to hit when on mobile.

Here's what I have so far, is anything missing? (The filter count badge only shows up above a certain width so it doesn't make the button touch the edge of the box)

flip1995 · 2022-05-02T10:32:15Z

I'll try that out, but I'm worried that'll make the input really small and difficult to hit when on mobile.

The height of the text box doesn't change with this. As for the width, you can keep that at a certain minimum size. I think always having to type out 1. and .0 even though this is always the same (for the foreseeable future) is worse from a UX perspective.

Serial-ATA · 2022-05-02T11:00:17Z

Makes sense 👍 I'll send a demo on Zulip by tomorrow.

xFrednet

I've looked over a previous version, these comments should still be valid, though. I'll do a final review soon (Currently, I'm a bit sick 😅 )

util/gh-pages/index.html

xFrednet · 2022-05-11T20:50:21Z

Thank you for your patience, my week has been very busy. This is definitely on my todo list for the weekend. 🙃

xFrednet

Overall looks good to me, note that I'm not a JS expert ^^ I've marked some smaller nits. Also, another visual NIT: When all three input fields have a value, the counter only says (2). IMO it should say false or mark one filter as invalid/not applied.

Other than that, it's a nice improvement 🙃

xFrednet · 2022-05-14T14:13:18Z

util/gh-pages/index.html

+            #version-filter-count {
+                display: none;
            }


I think it looks a bit weird, that only the version number disappears. I also feel like a with of 412 is super rare. My browser didn't allow making the window that small. Therefore, I would remove this css rule :)

I was testing how it looked on mobile through Firefox dev tools, and that was the width of the first device I saw. I don't know how accurate it is, I just really didn't like seeing the filter count go out of bounds 😅.

I don't know any mobile phone that has such a small screen size. Therefore, I would drop this filter. In the worst case it'll wrap sooner :)

As it happens my phone (Nexus 5X) has a CSS width of 412px 😄

CSS pixels are multiplied by the device pixel ratio, so for phones the number will be smaller than you'd expect

Ahh, I forgot about that, thanks for pointing that out. In that case, I would still prefer the filters to wrap earlier rather than removing the count indicator. Especially since the other counters stay. Would that change be alright with you?

This is the 2 in the screenshot above? Yeah sounds good to me for consistency

Yeah, that's the 2 the other filters have a similar indicator :)

util/gh-pages/index.html

util/gh-pages/script.js

…Version`

util/gh-pages/script.js

Serial-ATA · 2022-05-15T15:33:16Z

Another solution for the width issue is removing the padding.

I think this looks a lot nicer than wrapping, what do you think?

util/gh-pages/script.js

xFrednet · 2022-05-15T16:02:38Z

I think this looks a lot nicer than wrapping, what do you think?

IDK, it's better than wrapping, but the removed padding is also a bit weird. I would be okay with it, if this is only used when the screen is too small

Serial-ATA · 2022-05-15T16:44:34Z

I shrunk the padding rather than remove it entirely. It'll wrap on widths < 405px now.

xFrednet · 2022-05-15T19:41:32Z

The implementation looks good to me. Note that the length modifier no longer works. If I add a number >= 100 it simply disables the filter without any warning. Do I also understand the counter correctly that <= and >= are counted as one? I would expect each of them to increase the counter :)

Serial-ATA · 2022-05-15T19:59:30Z

The count is only showing active filters, so since <= is invalid it's only going to count >=.

xFrednet · 2022-05-15T20:54:12Z

Ahh, is it possible to add some highlighting what filter is active?

Serial-ATA · 2022-05-16T14:02:24Z

I think that number of active filters is a good enough indicator, seeing it drop when you put something invalid in.

xFrednet · 2022-05-16T17:48:55Z

I don't find it that obvious TBH, I actually though that this was accidental. But I also feel like I'm a bit too nit-picky this is a solid improvement and I probably just went a bit overboard 😅

So thank you very much for this update and for addressing all my comments. I appreciate it! 🙃

Let's see if the deployment works as intended (testing in production, there was actually one time, where I broke the website for solid 12 Hours. I've messed something up and couldn't fix it since I didn't have r+ privileges, that was... fun. I'm getting sidetracked, Edit: I don't want to imply that this will break anything. This story just came to mind :)).

@bors r+

bors · 2022-05-16T17:48:56Z

📌 Commit f112e4d has been approved by xFrednet

bors · 2022-05-16T17:49:03Z

⌛ Testing commit f112e4d with merge 6e86ab0...

bors · 2022-05-16T18:06:17Z

☀️ Test successful - checks-action_dev_test, checks-action_remark_test, checks-action_test
Approved by: xFrednet
Pushing 6e86ab0 to master...

Serial-ATA added 2 commits April 26, 2022 18:21

Add version filtering option to the lint list

bdc8961

Move js out to its own file

06cc1ab

rust-highfive assigned xFrednet Apr 26, 2022

rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Apr 26, 2022

Make filters implicit; Update symbols

9173780

Serial-ATA added 2 commits May 3, 2022 18:12

Only require minor version

2c70af3

Change input placeholder

dd7bc86

xFrednet reviewed May 5, 2022

View reviewed changes

util/gh-pages/index.html Outdated Show resolved Hide resolved

util/gh-pages/index.html Outdated Show resolved Hide resolved

util/gh-pages/index.html Outdated Show resolved Hide resolved

Use CamelCase; Fix filter ranges

4ed52bf

xFrednet reviewed May 14, 2022

View reviewed changes

Switch input to type=number; Separate version validation out of `by…

a30587e

…Version`

xFrednet reviewed May 15, 2022

View reviewed changes

util/gh-pages/script.js Show resolved Hide resolved

Use early returns

b81d703

xFrednet reviewed May 15, 2022

View reviewed changes

util/gh-pages/script.js Outdated Show resolved Hide resolved

util/gh-pages/script.js Outdated Show resolved Hide resolved

Improve appearance on smaller screens

f112e4d

xFrednet approved these changes May 16, 2022

View reviewed changes

bors merged commit 6e86ab0 into rust-lang:master May 16, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add version filtering option to the lint list #8752

Add version filtering option to the lint list #8752

Serial-ATA commented Apr 26, 2022

flip1995 commented Apr 28, 2022 •

edited

Loading

xFrednet commented Apr 28, 2022 •

edited

Loading

flip1995 commented Apr 28, 2022 •

edited

Loading

Serial-ATA commented Apr 28, 2022

xFrednet commented Apr 28, 2022

Serial-ATA commented Apr 28, 2022

xFrednet commented Apr 28, 2022

xFrednet commented Apr 28, 2022

flip1995 commented Apr 28, 2022

Serial-ATA commented Apr 28, 2022

flip1995 commented May 2, 2022 •

edited

Loading

Serial-ATA commented May 2, 2022

xFrednet left a comment

xFrednet commented May 11, 2022

xFrednet left a comment

xFrednet May 14, 2022

Serial-ATA May 14, 2022

xFrednet May 14, 2022

Alexendoo May 14, 2022

xFrednet May 15, 2022

Alexendoo May 15, 2022

xFrednet May 15, 2022

Serial-ATA commented May 15, 2022

xFrednet commented May 15, 2022

Serial-ATA commented May 15, 2022

xFrednet commented May 15, 2022

Serial-ATA commented May 15, 2022

xFrednet commented May 15, 2022

Serial-ATA commented May 16, 2022

xFrednet commented May 16, 2022 •

edited

Loading

bors commented May 16, 2022

bors commented May 16, 2022

bors commented May 16, 2022

Add version filtering option to the lint list #8752

Add version filtering option to the lint list #8752

Conversation

Serial-ATA commented Apr 26, 2022

flip1995 commented Apr 28, 2022 • edited Loading

xFrednet commented Apr 28, 2022 • edited Loading

flip1995 commented Apr 28, 2022 • edited Loading

Serial-ATA commented Apr 28, 2022

xFrednet commented Apr 28, 2022

Serial-ATA commented Apr 28, 2022

xFrednet commented Apr 28, 2022

xFrednet commented Apr 28, 2022

flip1995 commented Apr 28, 2022

Serial-ATA commented Apr 28, 2022

flip1995 commented May 2, 2022 • edited Loading

Serial-ATA commented May 2, 2022

xFrednet left a comment

Choose a reason for hiding this comment

xFrednet commented May 11, 2022

xFrednet left a comment

Choose a reason for hiding this comment

xFrednet May 14, 2022

Choose a reason for hiding this comment

Serial-ATA May 14, 2022

Choose a reason for hiding this comment

xFrednet May 14, 2022

Choose a reason for hiding this comment

Alexendoo May 14, 2022

Choose a reason for hiding this comment

xFrednet May 15, 2022

Choose a reason for hiding this comment

Alexendoo May 15, 2022

Choose a reason for hiding this comment

xFrednet May 15, 2022

Choose a reason for hiding this comment

Serial-ATA commented May 15, 2022

xFrednet commented May 15, 2022

Serial-ATA commented May 15, 2022

xFrednet commented May 15, 2022

Serial-ATA commented May 15, 2022

xFrednet commented May 15, 2022

Serial-ATA commented May 16, 2022

xFrednet commented May 16, 2022 • edited Loading

bors commented May 16, 2022

bors commented May 16, 2022

bors commented May 16, 2022

flip1995 commented Apr 28, 2022 •

edited

Loading

xFrednet commented Apr 28, 2022 •

edited

Loading

flip1995 commented Apr 28, 2022 •

edited

Loading

flip1995 commented May 2, 2022 •

edited

Loading

xFrednet commented May 16, 2022 •

edited

Loading