Closed mlissner closed 1 year ago
Performance performance performance -
The latest list:
?courts=abc,foo,bar
(right now it returns 413 responses b/c the requests are too big.
class="lead"
to the first paragraphif query_parameters
from the view.[x] The X button is weird in Florida:
what is weird about the x button?
It's all the way down by the line?
Making progress! Amazing how hard a single page can be, but I think this is an important one to get right, so here's what I hope is my last fine-tooth combing through it:
[x] The TOC at /coverage/
needs a tweak to remove updated stuff:
[x] We can remove the old JS library that was building the old charts, right?
[x] The coverage page has:
We have over 9 million opinions for over 2000 courts. See our page on this topic for more details:
Which ends with a colon. The same paragraph for the Financial Disclosures and Judges blurbs end with periods. Let's make it consistent with the period. Details, sorry!
[x] Can we remove this sentence:
Although the collection of legal opinions never stops, here is what we have and how we do it.
And replace it with a newline, and then:
Scroll down or use the Table of Contents to inspect particular jurisdictions or read on to learn more about how we do it.
[x] Let's switch the colons on the coverage page to m-dashes. So instead of "Diverse Sources: Our", let's do, "Diverse Sources — Our".
[x] There are a bunch of links in the coverage page google doc. Can you move them to the live version, please?
[x] Tweaks:
[x] The performance is....so-so. If it's cached, it's fast. NY just took 22s, which is waaaay longer than most users will wait. I think you can fix this by checking which courts we have scrapers for.
So instead of querying clusters to find out we have no scraped content, query the Court table, and only query clusters for dates if we have scrapers for that Court. Even North Dakota gets the :turtle: emoji in Firefox's network panel, since it takes over 1s. We can also make the cache a lot longer.
It's one day now. How about a week?
[x] Ohio is intense! But it still has some layout problems:
I think that's it. I think all of this is easy except, perhaps, the last item, which I'm not sure about. Hopefully we're on the final bit of this one, but not yet at the ends of our rope.
I dont have the layout issue but it's a width thing when you load the page. The chart is surprisingly not dynamic. I'll see what I can do
I've got some updates for mobile - I think but I dont have enough data to validate the Ohio NY situation yet.
Here is how mobile looks in safari in different form factors. The only thing holding this up is the fact that we need to complete our court citation string to use as a shortened string to make it work across all mobile options.
@mlissner
That's cool. I'm not sure the FB, F, etc. mean much to anybody though. Could we just remove those?
I was going to add a legend ? but we could easily remove them
I'd suggest removing them on xs, sm, and md, but leave them on lg?
I guess if we wanted to do a legend (and it's easy), we could use color of the bars instead and drop the left sidebar completely.
We need to update all citation string but otherwise it's great
To be clear it says federal appellate etc on big screens.
I mean, if you're doing a legend, is it easier to just ditch the left sidebar completely on all screensizes?
@mlissner the PR is ready for your review - ditched the legend - but made some other choices
OK, we're certainly getting there. I checked off a bunch of the boxes on this issue.
Ohio is still a little weird:
It's readable now, so it's probably fine to leave well enough alone, but I thought I'd raise it, since I know you were calculating widths and it surprised me that it had such a large margin on the right.
This is also really hard to understand. Why are there black and purpose boxes? What could it mean?
I'm not sure we can do much more about performance, so I guess that's done.
So, summarizing:
Whoa - Ohio is super weird on your screen. @ERosendo any chance you are having the same issue as @mlissner
I just checked the coverage page and I'm not seeing that result. 🤔
I'm getting this chart for Ohio:
@mlissner I've tested this on Safari, Firefox, Mobile Safari and Chrome - and do not see those results.
Hm, it works for me on Chrome, but not Firefox. I did a SHIFT+R refresh, but that didn't change anything (nor did I expect it to with our cache-busting approach). I don't know why it'd do that, but if it's just a me on Firefox/Linux thing, I think we can leave it be and focus on the rest.
The Black vs Purple is an artifact that distinguishes scrapers from non scrapers charts. It is caused by the chart treating the two rows differently because there is no value for the number of opinions that are represented there.
Since we are currently having an issue with the opinion / cluster totals - dropping the value for the number of opinions will change every line to black.
OK, but if every line is black, how will you know that lines 3 and 4 of my screenshot above are for scrapers and lines 1 and 2 are not?
You won't on mobile - but they will be listed as such on larger screen sizes. Alternatively we could add a (Scrp) or something to the label on the right?
Additionally
<script type="text/javascript" nonce="{{ request.csp_nonce }}">
var precedentTypes = [
{% for status in precedential_statuses %}
'{{ status }}'{% if not forloop.last %},{% endif %}
{% endfor %}
];
var sorted_courts = {{ sorted_courts|safe}};
</script>
I believe this can be removed as well - as it creates variables that were used in the coverage.js file that was removed
Alternatively we could add a (Scrp) or something to the label on the right?
Easy enough and better than nothing. Sure.
OK, I think we can close this one with #3254
A few things I"m seeing and thinking about wrt to the new coverage page:
[x] Performnace: Right now loading the page itself seems to be solved, but loading the graphs is bad. I don't think we can fix it using the database, but fear not, there are solutions:
[x] Mobile: The sidebar is missing on mobile, which is too bad, and the graphs don't work. I think we can just hide the graph buttons on mobile (sorry), and maybe we can do a mini-TOC that only shows up on mobile. It could just have State, Federal, etc. headings.
[x] Architecture: I didn't catch it before now, but the API shouldn't take the name of the court as a parameter. It should use IDs. Names shouldn't be used for this kind of thing.
[x] Content: We need to add the text from the Google doc.
What else?