freedomofpress / securethenews

An automated scanner and web dashboard for tracking TLS deployment across news organizations
https://securethe.news
GNU Affero General Public License v3.0
102 stars 25 forks source link

Add leaderboards by regions #143

Closed redshiftzero closed 5 years ago

redshiftzero commented 6 years ago

Fixes #91

screen shot 2017-11-22 at 11 26 45 pm screen shot 2017-11-25 at 12 56 08 am screen shot 2017-11-25 at 1 02 01 am screen shot 2017-11-25 at 12 55 46 am screen shot 2017-11-25 at 3 08 06 am
SaptakS commented 6 years ago

Looks Awesome!! I feel the categories in View Leaderboards by Category would look better in a 3 grid type view? In a way tags are shown? Seems like too much white space on the left and right to me. Apart from that, looks really great

redshiftzero commented 6 years ago

Thanks for reviewing this @SaptakS :heart:!

Yeah you're right, there's definitely too much whitespace... do you mean something kinda like this:

screen shot 2018-06-29 at 4 58 49 pm

I thought about including the little flag icons but I think that would look pretty busy. The above would work provided the number of leaderboards is small, else eventually a dropdown menu might be better.

SaptakS commented 6 years ago

The above would work provided the number of leaderboards is small, else eventually a dropdown menu might be better.

I completely agree. If it goes to more than 3 rows, then a dropdown would be much better.

SaptakS commented 5 years ago

I have modified the frontend such that if there are less than 5 categories, it just shows them in a flex. If there is more than 5 elements, it shows the first 5 elements in a flex with a "Show more" button below. On clicking "Show more" it shows the rest of the categories in a flexbox below the 1st 5 elements with smaller button sizes.

Show more: Screenshot from 2019-08-23 15-03-56

With all the categories: Screenshot from 2019-08-23 15-04-00

I think another good modification might be getting the categories sorted by the number of sites they have, such that first 5 categories would then denote the 5 categories with most sites under them.

cc: @redshiftzero @harrislapiroff

eloquence commented 5 years ago

Per discussion today, @thisisparker will take a spin through this sometime this week as well, please wait for his input prior to merge :)

conorsch commented 5 years ago

One comment on the implementation here: we have a single category per news website. Right now, we intend to use that to define "regions", but it's conceivable we'd want a more flexible solution that would allow sorting based on coverage type (e.g. government-accountability, national-security, technology, etc.). The regions-based views have been in high enough demand that I'd be happy merging as-is, but would appreciate thoughts from @SaptakS or @harrislapiroff on any complications to migrate to a more flexible system in the future if we choose to.

SaptakS commented 5 years ago

@conorsch I think to implement a more flexible solution we need to add an Association to implement many-to-many relationship between site_categories and site_pages. Right now, it's a one-to-many relationship. But I think migration shouldn't be that painful. @harrislapiroff and @conorsch, I can try and implement that in this PR itself.

thisisparker commented 5 years ago

This looks really good, and it's an exciting and much-requested feature! I wrote up some notes as I was checking it out, which might be useful for thinking about future development, but I don't think any of them are blockers.

But all in all, very nice! And the links that are there looks very slick.

eloquence commented 5 years ago

Thanks, @thisisparker! We discussed this in web sync today. @SaptakS will aim to update the API in this PR, and we can track other improvements in follow-up issues. I'll give this a spin as well, and then we can all sync up on how we want to deploy the first version of this change (ideally with a blog post, and some first new sites imported).

eloquence commented 5 years ago

One comment on the implementation here: we have a single category per news website. Right now, we intend to use that to define "regions", but it's conceivable we'd want a more flexible solution that would allow sorting based on coverage type (e.g. government-accountability, national-security, technology, etc.).

I think ideally we'll end up with something similar to the SecureDrop directory filters:

https://securedrop.org/directory/

Note that country/regions and topics are intentionally separate, to make it easier to navigate both lists. ("France" and "Germany" should not be stuck between "Environment" and "Health"). Still, both are many-to-many relationships: one site can have multiple countries, and multiple topics.

Upon consideration, I would argue that there are two changes we should make before merging this PR:

Upon deployment, we can then start tagging news sites with countries/regions and add some requested sites. We would reserve the following changes for future iterations:

SaptakS commented 5 years ago

@harrislapiroff @eloquence I have made the above mentioned changes. It's a many to many field now. To avoid confusion I have named it everywhere is region so that the intent is clear instead of more generic categories. The admin sections for sites with wagtail-autocomplete(finally) looks something like this:

Screenshot-2019-9-12 Wagtail - Editing site

SaptakS commented 5 years ago

@chigby made the changes, fixed the panels and made the design so that there is same amount of padding below both in case of less than 5 regions and more.

eloquence commented 5 years ago

Agree with Cameron's comments, and would also recommend really doing a scrub for "categories"->"regions" through this entire PR. These types of terminology inconsistencies are IMO best resolved during development, as they are otherwise likely to cause confusion later.

Otherwise this looks great for the first deployment of this feature. :)

SaptakS commented 5 years ago

@eloquence @chigby done!!