Skip to content
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

Category dashboard with collapsible filters #934

Open
wants to merge 24 commits into
base: main
Choose a base branch
from

Conversation

sarahfossheim
Copy link
Collaborator

@sarahfossheim sarahfossheim commented Sep 24, 2024

New features

  • New compact header only used on the tech report pages
  • Sidebar with filters, instead of big card on top of the page
  • Prototype of the categories page:
    • Table with summarising info on all techs within that category
    • Possibility to click through to a specific tech, or compare multiple (or all) technologies within the tech
    • Prototype sortable table functionality (not in the UI yet)
    • /reports/techreport/category
  • Only one view for both drilldown and comparison view
    • Add more technologies to compare in the sidebar
    • New template loads depending on the amount of techs selected
    • See /reports/techreport/tech

Bugfixes

  • Updates to the responsiveness/zoom (more improvements to come in future PR)
  • Debugging bright yellow for WordPress line changed to color with enough contrast
  • Updated copywriting for some section (more improvements to come in future PR)
  • Fixed keyboard order for the points on comparison graphs (it was reversed)

src/js/components/filters.js Fixed Show fixed Hide fixed
src/js/techreport/index.js Fixed Show fixed Hide fixed
@rviscomi rviscomi marked this pull request as draft October 9, 2024 13:47
@sarahfossheim sarahfossheim marked this pull request as ready for review November 4, 2024 23:04
@sarahfossheim sarahfossheim changed the title [WIP] Category dashboard with collapsible filters Category dashboard with collapsible filters Dec 4, 2024
@rviscomi rviscomi requested a review from tunetheweb December 4, 2024 15:14
Copy link
Member

@rviscomi rviscomi left a comment

Choose a reason for hiding this comment

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

Staged here and left a bunch of feedback: https://technology-report-dot-httparchive.uk.r.appspot.com/reports/techreport/category?geo=ALL&rank=ALL&category=CMS

I had to patch one bug to get it to work (the fix is in one of the comment suggestions) but otherwise these are all just feature requests. Great work!

const bLatest = bSortedDate[0];

// Get the correct endpoint & metric
const aMetric = aLatest[sortEndpoint].find(row => row.name === sortMetric);
Copy link
Member

Choose a reason for hiding this comment

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

I'm getting an error on this line:

Uncaught (in promise) TypeError: Cannot read properties of undefined (reading 'adoption')


console.log(sortMetric);
if(sortMetric) {
this.dataArray = this.dataArray.sort((techA, techB) => {
Copy link
Member

Choose a reason for hiding this comment

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

I was able to work around the error below by filtering out empty arrays before sorting. Not sure if it's the best fix though.

Suggested change
this.dataArray = this.dataArray.sort((techA, techB) => {
this.dataArray = this.dataArray.filter(tech => tech.length).sort((techA, techB) => {

"type": "heading"
},
{
"key": "origins",
Copy link
Member

Choose a reason for hiding this comment

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

Values appear as 3222201 which can be harder to read. Could we format with commas, ie 3,222,201? It'd also be helpful for comparing values if the text in this column is right-aligned, like a spreadsheet.

src/js/techreport/tableLinked.js Outdated Show resolved Hide resolved
});
}

this.dataArray.forEach(technology => {
Copy link
Member

Choose a reason for hiding this comment

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

I know we talked about pagination before, but I don't remember if we talked about performance improvements to the initial API requests.

So for example if we only want to show the top 10 technologies by default and we can assume that the categories API endpoint lists the technologies in descending popularity order, we could make requests to the other API endpoints with only those first 10 technologies as request params to improve performance a bit.

"type": "heading"
},
{
"key": "origins",
Copy link
Member

Choose a reason for hiding this comment

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

When filtering to the top 1k rank, I'm seeing technologies with "null" origin counts

https://technology-report-dot-httparchive.uk.r.appspot.com/reports/techreport/category?tech=&geo=ALL&rank=Top+1k&category=CMS

image

Comment on lines +50 to +54
// Sort techs by date to get the latest
const aSortedDate = techA.sort((a, b) => new Date(b.date) - new Date(a.date));
const bSortedDate = techB.sort((a, b) => new Date(b.date) - new Date(a.date));
const aLatest = aSortedDate[0];
const bLatest = bSortedDate[0];
Copy link
Member

Choose a reason for hiding this comment

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

"Latest" should be the same for all technologies. It's possible for a technology to exist at one point in the dataset and then disappear forever, but this would always compare that ancient technology against the latest versions of all other modern technologies.

For example, October CMS shows up at the top of the top 1k CMS list with 361 origins:

https://technology-report-dot-httparchive.uk.r.appspot.com/reports/techreport/category?tech=&geo=ALL&rank=Top+1k&category=CMS

image

But in the technology drilldown it's clearer that the most recent (and only) data point is from April 2022:

https://technology-report-dot-httparchive.uk.r.appspot.com/reports/techreport/tech?tech=October%20CMS&geo=ALL&rank=Top%201k

image

The expected behavior is for it to not appear on the Category page at all since it's not part of the latest dataset. That might also be a good reason to explicitly have a label like Latest data: 2024-11-01 at the top of this report, similar to the others.

@@ -1,18 +1,18 @@
<div class="tech-selector-group" data-tech="{{ name }}">
<div class="content">
<div class="categories-selector-group">
<!-- <div class="categories-selector-group">
Copy link
Member

Choose a reason for hiding this comment

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

This HTML is commented out. Should it be removed?

<label for="categories_selector" class="tech">Category</label>
<div class="tech-input-wrapper">
{% set category_selected = tech_report_page.filters.category or tech_report_config.default_category %}
<select name="categories" class="tech" data-selected="{{ category_selected }}" id="categories_selector">
Copy link
Member

Choose a reason for hiding this comment

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

I'm finding the native<select>hard to use for such long lists of options. Do you have any thoughts on using a datalist instead? I like having the ability to start typing what I'm looking for and it can match any position in the value, whereas typing in a <select> only works if it's a prefix match, and the datalist also filters down the results to make it easier to find what you're looking for.

Here's a production example: https://chromestatus.com/metrics/feature/popularity

image

Maybe something to consider for the technologies inputs too.

{% set table = component.table %}
{% include "techreport/components/table_linked.html" %}

<a data-name="selected-apps" href="/reports/techreport/tech" class="cta-link">Compare technologies</a>
Copy link
Member

Choose a reason for hiding this comment

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

If there are geo/rank/client filters set on the Categories page and I click this to compare multiple technologies, it looks like those settings are lost on the comparison page.

USA, Top 100k, Desktop:

image

All, All, mobile:

image

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