Add vaccine type filter, move all filters to separate component #13

linusmarco · 2021-02-13T21:04:55Z

This PR:

Adds a vaccine type filter (Implements Allow filtering to type of vaccine #4)
Moves the added filter and the existing availability filter to their own component/panel
- This new panel lives on the left side of the screen for md+ size screens (desktop, probably) and at the top on smaller screens (mobile, probably)
- On larger screens, the filters are sticky so that they follow when the user scrolls down
Removes the .eslintcache file from version control
Fixes a few React warnings

Larger screen:

Smaller screen:

harcod · 2021-02-13T21:45:12Z

Bug Report: "Amherst Regional High School" gets replicated in the listing when switching back and forth between "Pfizer" and "Moderna" filter. In addition, the replicas don't have any "More Information" attached to them. Please note that the current data feed has two separate cards for "Amherst Regional High School" even on the live site.

jrogers97 · 2021-02-13T22:45:35Z

src/components/FilterPanel.js

+ useEffect(() => {
+ onChange({
+ hasAppointments: hasAppointments ?
+ d => !!d.hasAppointments === hasAppointments


Isn't this function essentially the same as d => !!d.hasAppointments? Because it's only used when hasAppointments is true, so !!d.hasAppointments === hasAppointments is always just checking that !!d.hasAppointments is true.

Also, to avoid the ternary, this whole value could be simplified to hasAppointments: d => !hasAppointments || !!d.hasAppointments

Good point. This was some real galaxy brain code...

jrogers97 · 2021-02-13T22:49:29Z

src/components/FilterPanel.js

+ d => !!d.hasAppointments === hasAppointments
+ :
+ () => true,
+ vaxType: vaxType !== 'Any' ?


Same idea here, I think the ternary is slightly harder to understand and could be replaced with vaxType: d => vaxType === 'Any' || (d.extraData && d.extraData['Vaccinations offered'] && d.extraData['Vaccinations offered'].includes(vaxType)).

I guess it comes down to preference, maybe the one-liner is harder to read, in which case ignore me!

To me this feels a little less transparent, but if others prefer this way too I could change it.

I think pretty much any use of the ternary operator that isn't for short simple expressions should be avoided, and it definitely shouldn't be combined with arrow functions. I would move the test into the function, and further simplify a bit:

d => { if (vaxType !== 'Any') { let vaxOffered = d.extraData && d.extraData['Vaccinations offered']; return vaxOffered && vaxOffered.includes(vaxType); } else { return true; }

(untested, but I think that's equivalent)

Just as you have an aversion to ternary operators, I have one for assignment expressions with && or ||. They always require a second look. As a compromise, how about we go fully explicit?

vaxType: d => { if (vaxType === 'Any') { return true; } else { if (d.extraData && d.extraData['Vaccinations offered']) { return d.extraData['Vaccinations offered'].includes(vaxType); } else { return false; } } }

Err, to be clear, the concern is not the ternary operator — it's the ternary expression being split over 9 lines that include an arrow function, where the test is not obviously better outside the arrow function than in.

Why is && ok in

if (d.extraData && d.extraData['Vaccinations offered']) {

but not in

return vaxOffered && vaxOffered.includes(vaxType);

?

Also, part of the point of the simplification (and introduction of let vaxOffered) was to avoid searching the d.extraData dictionary twice (and cut down line length and cognitive load). Yours undoes that.

If you really like ?: and dislike && I'd replace

return vaxOffered && vaxOffered.includes(vaxType);

with

return vaxOffered ? vaxOffered.includes(vaxType) : false;

which, although silly to my eye, would seem better than having an entire 5-line if/else block to accomplish the same thing.

But I am In No Way King, and your revised solution eliminates the mass confusion point of the long ternary-operator-with-arrow-function-within, so is definitely an improvement over the original.

I'm not trying to pick a fight here, man--just addressing readability. I'll go with the most recent revision with the explicit if-else's so we're not running in circles.

linusmarco · 2021-02-14T02:36:46Z

Bug Report: "Amherst Regional High School" gets replicated in the listing when switching back and forth between "Pfizer" and "Moderna" filter. In addition, the replicas don't have any "More Information" attached to them. Please note that the current data feed has two separate cards for "Amherst Regional High School" even on the live site.

I think the only real solution to this issue is to resolve this on the back end by giving each site a unique ID. Below are the two sites causing problems. The issue seems to be that they have a morning and an evening clinic listed separately so the key based on the basic site info (name, city) was not unique. For now I generated a key for each site based on the site's whole object. Not ideal for efficiency reasons but it will work for now.

        {
            "name": "Amherst Regional High School",
            "street": "21 Matoon St",
            "city": "Amherst",
            "zip": "01002",
            "availability": {
                "02/18/2021": {
                    "hasAvailability": true,
                    "numberAvailableAppointments": 365,
                    "signUpLink": "https://www.maimmunizations.org/client/registration?clinic_id=1070"
                }
            },
            "hasAvailability": true,
            "extraData": {
                "Vaccinations offered": "Pfizer-BioNTech COVID-19 Vaccine",
                "Age groups served": "Adults, Seniors",
                "Services offered": "Vaccination",
                "Clinic Hours": "09:00 am - 11:45 am"
            }
        },

and

        {
            "name": "Amherst Regional High School",
            "street": "21 Mattoon Street",
            "city": "Amherst",
            "zip": "01002",
            "availability": {
                "02/17/2021": {
                    "hasAvailability": true,
                    "numberAvailableAppointments": 229,
                    "signUpLink": "https://www.maimmunizations.org/client/registration?clinic_id=1055"
                }
            },
            "hasAvailability": true,
            "extraData": {
                "Vaccinations offered": "Pfizer-BioNTech COVID-19 Vaccine",
                "Age groups served": "Adults, Seniors",
                "Services offered": "Vaccination",
                "Clinic Hours": "01:00 pm - 02:45 pm"
            }
        },

harcod · 2021-02-14T15:10:00Z

Bug Report: "Amherst Regional High School" gets replicated in the listing when switching back and forth between "Pfizer" and "Moderna" filter. In addition, the replicas don't have any "More Information" attached to them. Please note that the current data feed has two separate cards for "Amherst Regional High School" even on the live site.

Clarification and Steps to Reproduce Bug:

Load the page the first time. The vax type "Any" is selected by default.
There are two "Amherst Regional High School" entries in the list. This is correct because they are actually different times.
Select filter Vax Type = "Pfizer".
Now there are three "Amherst Regional High School" entries in the list.
Select filter Vax Type = "Moderna".
Now there are two "Amherst Regional High School" entries in the list. Neither of these two entries have any "More Information" available.
Select filter Vax Type = "Pfizer"
Now there are four "Amherst Regional High School" entries in the list.

If you continue to switch back and forth, more and more entries are created for "Amherst" in the listing.

linusmarco · 2021-02-14T15:27:56Z

Bug Report: "Amherst Regional High School" gets replicated in the listing when switching back and forth between "Pfizer" and "Moderna" filter. In addition, the replicas don't have any "More Information" attached to them. Please note that the current data feed has two separate cards for "Amherst Regional High School" even on the live site.

Clarification and Steps to Reproduce Bug:

Load the page the first time. The vax type "Any" is selected by default.
There are two "Amherst Regional High School" entries in the list. This is correct because they are actually different times.

Select filter Vax Type = "Pfizer".
Now there are three "Amherst Regional High School" entries in the list.

Select filter Vax Type = "Moderna".
Now there are two "Amherst Regional High School" entries in the list. Neither of these two entries have any "More Information" available.

Select filter Vax Type = "Pfizer"
Now there are four "Amherst Regional High School" entries in the list.

If you continue to switch back and forth, more and more entries are created for "Amherst" in the listing.

@harcod are you still seeing this after the most recent commits to address the issue? This was caused by dupe keys. See my previous comment on how I fixed it by creating a unique key. I don't see this bug after 158e646, but please let me know if you still do

harcod · 2021-02-14T16:11:18Z

@linusmarco Fixed now. ( Sorry. For some reason, I didn't have that one update the fixed dupe keys. )

livgust · 2021-02-16T19:27:02Z

OK so I mentioned this morning that I'm a little nervous about the design, since we have proximity filtering coming through soon too as well. What do you think about this? It's a common design pattern on other websites with filtering.
(Also ignore my bad Paint skills, replace [] with checkboxes, () with radio buttons, and use Material UI text styling)

Desktop:

Mobile:

livgust · 2021-02-16T19:29:06Z

I like this component for a side bar - it's basically the same as what you did but it looks a little more "separate" which I like. Thoughts?

livgust · 2021-02-16T19:03:02Z

src/components/FilterPanel.js

+ })
+ }, [onChange, hasAppointments, vaxType])
+
+ const vaxTypes = Array.from(


I'm a little nervous here. Only the MA vaccination sites will have that "Vaccinations offered" information that we scrape up; no other sites will have it. Therefore by default we'll show them in the "Any" bucket which is misleading. It might be worth entering manually what sites (outside of the state sites) have which vaccines, but then that's something we would have to manually update if it ever changes.

Curious if you (or anyone else) have thoughts.

…arco/macovidvaccines.com into filter-updates

* Update docs to make PRs to develop * update develop again (livgust#32) * Improve HTML semantics of a couple key elements * Tweaks to elements * Add eligibility accordion * Remove extraneous prettierrc and prettify README.md * Link to mass.gov/covidvaccine gave a security warning becuase the state didn't redirect properly. Link changed to a non-redirecting link. * Update eligibility for feb 18, small refactor * link to senior housing details Co-authored-by: Matthew Cardarelli <mattrc7@gmx.com> Co-authored-by: Jake Rogers <jake@dataplusmath.com> Co-authored-by: Rob Harris <rob@harcod.com> Co-authored-by: John Hawkinson <jhawk@alum.mit.edu> * get filters into responsive drawer Co-authored-by: Olivia Adams <liv.gust@gmail.com> Co-authored-by: Matthew Cardarelli <mattrc7@gmx.com> Co-authored-by: Jake Rogers <jake@dataplusmath.com> Co-authored-by: Rob Harris <rob@harcod.com> Co-authored-by: John Hawkinson <jhawk@alum.mit.edu>

linusmarco · 2021-02-19T02:21:45Z

This has now been updated to use the new responsive drawer for the filters and now uses the checkbox-based design that @livgust proposed:

Desktop:

Mobile:
(filter panel closed)

(filter panel open)

livgust · 2021-02-19T21:24:42Z

LOVE THIS so far!!!

I think next steps are:

confirming design with Daniel Cho (danieljaecho93)
adding proximity filtering (zip code search), which we already have some code for in another PR

livgust · 2021-02-19T21:24:57Z

In true Olivia fashion, I was unable to pick this up today. Womp womp.

linusmarco · 2021-02-20T19:37:43Z

@danieljaecho93, thoughts on the design here?

EDIT: sorry I hadn't refreshed and seen that this was merged...ignore me

danieljae · 2021-02-22T14:40:54Z

@linusmarco Looks great :) Something to consider on my end is what sorts of different categories do we want to display for the filters-for instance, I'm wondering how much people would want to filter for places with no available appointments? But I will do some research on my own and look into that, this will definitely work for the time being!

linusmarco added 2 commits February 13, 2021 15:55

remove .eslintcache from tracking

30587da

Add vaccine type filter (livgust#4), move all filters to panel

07eebe6

linusmarco requested a review from livgust as a code owner February 13, 2021 21:04

jrogers97 reviewed Feb 13, 2021

View reviewed changes

address code review comments

158e646

more code review revisions

a10bb88

remove console.log

f8d24fa

livgust requested changes Feb 16, 2021

View reviewed changes

livgust mentioned this pull request Feb 16, 2021

Ranking by Location Distance #15

Closed

livgust and others added 11 commits February 17, 2021 11:37

Merge branch 'feature/4-filter-vax-type' of https://github.com/linusm…

45e1d7b

…arco/macovidvaccines.com into filter-updates

WIP updating tests - one still failing

7caae01

Add warning when sites are restricted

8c6a9c8

moving to new line

c6c034c

Merge branch 'show-restrictions' into filter-updates

4751973

fixed test

1238db4

added missing comma, womp womp

8c7493c

merge changes from develop

32df8c6

get filters into responsive drawer

b49fa2c

Merge branch 'develop' into filter-updates

b4040af

harcod linked an issue Feb 18, 2021 that may be closed by this pull request

Allow filtering to type of vaccine #4

Open

linusmarco added 2 commits February 18, 2021 19:58

merging changes from main repo

11fe7a0

get new filter panel checkbox design working

ccdda03

linusmarco changed the base branch from master to develop February 19, 2021 02:16

merge in most recent changes from develop

a0d5fc8

livgust merged commit fda595e into livgust:develop Feb 19, 2021

johnhawkinson mentioned this pull request Mar 5, 2021

Merge develop into master, with Rob's sort-zipcode #115

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add vaccine type filter, move all filters to separate component #13

Add vaccine type filter, move all filters to separate component #13

linusmarco commented Feb 13, 2021 •

edited

Loading

harcod commented Feb 13, 2021

jrogers97 Feb 13, 2021

linusmarco Feb 14, 2021

linusmarco Feb 14, 2021

jrogers97 Feb 13, 2021

linusmarco Feb 14, 2021

johnhawkinson Feb 14, 2021

linusmarco Feb 14, 2021

johnhawkinson Feb 14, 2021

linusmarco Feb 14, 2021

linusmarco commented Feb 14, 2021

harcod commented Feb 14, 2021

linusmarco commented Feb 14, 2021

Clarification and Steps to Reproduce Bug:

harcod commented Feb 14, 2021

livgust commented Feb 16, 2021

livgust commented Feb 16, 2021

livgust Feb 16, 2021

linusmarco commented Feb 19, 2021

livgust commented Feb 19, 2021

livgust commented Feb 19, 2021

linusmarco commented Feb 20, 2021 •

edited

Loading

danieljae commented Feb 22, 2021

Add vaccine type filter, move all filters to separate component #13

Add vaccine type filter, move all filters to separate component #13

Conversation

linusmarco commented Feb 13, 2021 • edited Loading

harcod commented Feb 13, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

linusmarco commented Feb 14, 2021

harcod commented Feb 14, 2021

Clarification and Steps to Reproduce Bug:

linusmarco commented Feb 14, 2021

Clarification and Steps to Reproduce Bug:

harcod commented Feb 14, 2021

livgust commented Feb 16, 2021

livgust commented Feb 16, 2021

Choose a reason for hiding this comment

linusmarco commented Feb 19, 2021

livgust commented Feb 19, 2021

livgust commented Feb 19, 2021

linusmarco commented Feb 20, 2021 • edited Loading

danieljae commented Feb 22, 2021

linusmarco commented Feb 13, 2021 •

edited

Loading

linusmarco commented Feb 20, 2021 •

edited

Loading