Skip to content

Conversation

davehayes
Copy link

@davehayes davehayes commented Jun 15, 2022

This is to address issue #12 which I and one other ports maintainer had a need for. Let me know if this works for you and I will support this work on request.

Sample page: https://www.dream-tech.com/samplebycat.html

@davehayes
Copy link
Author

https://www.dream-tech.com/samplebycat.html ... forgot to mention I have a sample page up

Copy link
Member

@koobs koobs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for this @davehayes!

Initial review items (still need to do a deep dive):

Like other portscout pages [1]:

  • Have the portname link to the freshports page
  • Change field names to match existing "Port", "Current" and "New" fields in use elsewhere
  • Change 'New Versions X out of Y' to match existing "% Out of Date" values [2]
  • Match existing font styles, sizes and colours wherever possible
  • Add "Filter Category (regex):" functionality as on other pages, to filter/only show matching category entries.

Accordingly, as creative as it was, I'd like to drop the use of the [+] to expand/collapse from this PR. In its current form it doesn't match the 'show all by default' of the other pages, and has a minor UX issue (remains + even when expanded).

Happy to consider proposals to improve design and UI in a separate PR.

One other thought:

Might we want to have a page per category (category.html) with a summary page like the existing portscout homepage that has a line per maintainer with summary stats, that one can then click through to for the category run down.

This would allow us to retain the' X of Y ' numbers in this PR, have out ' % out of date ' summary while remaining consistent with, and using the existing page/list structures.

[1] https://portscout.freebsd.org/[email protected]
[2] https://portscout.freebsd.org/

- Portname links to the freshports page (with https)
- Table header field names changed to match portscout standards
- Changed X out of Y to print a percentage and "out of date" as a label
- CSS from templates pasted into new category template
- Accordion style UI removed in favor of a search box and visibility toggling via that

Note that I cannot ensure that styles are compatable when the CSS file containing the styles
isn't even a part of this repository. :)
@davehayes
Copy link
Author

Thank you for this @davehayes!

You are quite welcome. :)

Initial review items (still need to do a deep dive):

So ... I personally consider UI/UX issues to be quite subjective and
artistic. As such, and given that my visual artistic skills generally lack
consensual approval (and sometimes even my own approval ... heh), I don't
have much of an opinion on my own work in this area.

Now you folks have your own CSS file which I do not have. There are no
CSS files in the distribution. So when I did this I figured you would want to
include your own and my included CSS would be rendered irrelevant.

Nevertheless I do recognize the implied theme of your review, which is
essentially "make this look like the other portscout pages", and in the spirit
of getting this committed I'll happily make these changes.

What I am much more concerned about is what I did to pushrow() in your
template engine. I did manually test that I didn't affect currently generated
pages, but more eyes is always better. Please have a look at that first?

Like other portscout pages [1]:

  • Have the portname link to the freshports page

A minor nit that is not like the regular freshports pages, I made the links
'https' rather than 'http'. Was there a rationale for making the scheme
'http' on your pages? It's a simple edit either way.

  • Match existing font styles, sizes and colours wherever possible

I have re-pasted the CSS from the index.html template, and repurposed
resultsrowupdated for catrow, to distinguish category entries from ports
entries. I've also attempted to provide a rich set of classes for further
styling at least. Anything else is likely handled by your specific CSS file? :)
I'm open to suggestions if you find further flaw with the styling.

  • Add "Filter Category (regex):" functionality as on other pages, to filter/only show matching category entries.

I've added this, merging my way of doing things and this code's way. Let me
know if that works for you.

Accordingly, as creative as it was, I'd like to drop the use of the [+] to expand/collapse from this PR. In its current form it doesn't match the 'show all by default' of the other pages, and has a minor UX issue (remains + even when expanded).

Heh. I did it that way because of the original issue request in #12 where you
said:

Ability to 'open' ([+] toggle or similar) each category to see out of date ports in each

The UX issue is easily solved if you ever want that form.

Happy to consider proposals to improve design and UI in a separate PR.

As long as you understand I consider UI issues as subjective discussion, then
sure I'm willing to go through that. :) Really, just this list as I
am submitting is all I need. Now that I know your code a little better I'd
be willing to contibute some more. For example...

Might we want to have a page per category (category.html) with a summary
page like the existing portscout homepage that has a line per maintainer
with summary stats, that one can then click through to for the category run
down.

This is a good idea, but since we now have a search box which takes category
regular expressions, maybe it's too much? For me, it would be faster to have a
category dropdown from that filter box. As it is, if I wanted blazing speed
I'd simply search for the category name using whatever browser hot key finds
text in pages.

@davehayes davehayes requested a review from koobs June 16, 2022 21:47
@davehayes
Copy link
Author

Whoops, I just updated the sample page too so you can see it.

@koobs
Copy link
Member

koobs commented Jun 19, 2022

So ... I personally consider UI/UX issues to be quite subjective and artistic. As such, and given that my visual artistic skills generally lack consensual approval (and sometimes even my own approval ... heh), I don't have much of an opinion on my own work in this area.

Absolutely agree. Premise here just being lets split out design changes and do that in a separate piece of work.

Now you folks have your own CSS file which I do not have. There are no CSS files in the distribution. So when I did this I figured you would want to include your own and my included CSS would be rendered irrelevant.

Mmm, didn't realise that! Might be worth importing these, particularly if it helps facilitate your new functionality design, and helps us iterate on them. Thoughts @grembo ?

Nevertheless I do recognize the implied theme of your review, which is essentially "make this look like the other portscout pages", and in the spirit of getting this committed I'll happily make these changes.

Yep that's the basic gist. Certainly dont/didnt want to imply we dont want design improvements (we need them!) :D

What I am much more concerned about is what I did to pushrow() in your template engine. I did manually test that I didn't affect currently generated pages, but more eyes is always better. Please have a look at that first?

I don't currently have the cycles to re-ramp up on my perl and test this part of the review, so I'll need to lean on you and @grembo for that part, but I will certainly run through it and mention anything that stands out.

Like other portscout pages [1]:

  • Have the portname link to the freshports page

A minor nit that is not like the regular freshports pages, I made the links 'https' rather than 'http'. Was there a rationale for making the scheme 'http' on your pages? It's a simple edit either way.

Yep that's fine, and no, just historical and we've never changed them. Can we create a separate issue to bring all other url's into line as well?

  • Match existing font styles, sizes and colours wherever possible

I have re-pasted the CSS from the index.html template, and repurposed resultsrowupdated for catrow, to distinguish category entries from ports entries. I've also attempted to provide a rich set of classes for further styling at least. Anything else is likely handled by your specific CSS file? :) I'm open to suggestions if you find further flaw with the styling.

I don't mind how we go about solving this for this iteration, only goal being keep the design consistent for now, and if we can, make development/iterating on design easier for everyone. Duplicating code is not a dealbreaker/blocker for me, and Id rather lean toward progress and landing this.

  • Add "Filter Category (regex):" functionality as on other pages, to filter/only show matching category entries.

I've added this, merging my way of doing things and this code's way. Let me know if that works for you.

Trust you and @grembo on this.

Accordingly, as creative as it was, I'd like to drop the use of the [+] to expand/collapse from this PR. In its current form it doesn't match the 'show all by default' of the other pages, and has a minor UX issue (remains + even when expanded).

Heh. I did it that way because of the original issue request in #12 where you said:

Ability to 'open' ([+] toggle or similar) each category to see out of date ports in each

The UX issue is easily solved if you ever want that form.

My only thought was that if we add/deploy +/- toggles, it'd be great to do separately (as there's value for those all over).

Happy to consider proposals to improve design and UI in a separate PR.

As long as you understand I consider UI issues as subjective discussion, then sure I'm willing to go through that. :) Really, just this list as I am submitting is all I need. Now that I know your code a little better I'd be willing to contibute some more. For example...

Totally, key goals just being 1) split functionally/logically' features, and 2) new features added whole site

Might we want to have a page per category (category.html) with a summary
page like the existing portscout homepage that has a line per maintainer
with summary stats, that one can then click through to for the category run
down.

This is a good idea, but since we now have a search box which takes category regular expressions, maybe it's too much? For me, it would be faster to have a category dropdown from that filter box. As it is, if I wanted blazing speed I'd simply search for the category name using whatever browser hot key finds text in pages.

I'm easy, just thought I'd raise question as as its likely where its going to head anyway and its consistent with the existing organizational structure, so less confusing for users.

Having said that, if we're going to redesign, its very likely the existing information architecture will be redone entirely anyway.

Your call on effort:reward given the desire and high likelihood of wanting a total redesign

Thank you again for your effort :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants