opportunity-hack / evs

Equestrian Volunteer Scheduler
https://www.ohack.dev/nonprofit/89076bcc077811edbfdeb29c4ac23549
MIT License
1 stars 2 forks source link

Fix loading and editing roles #36

Closed parkerdavis1 closed 1 year ago

parkerdavis1 commented 1 year ago

Updated logic so that checkboxes show correct initial status for the various roles on the edit user page.

Updated the role connect and disconnect logic to allow for multiple roles.

Equality operator (==) swapped for Strict equality operator (===) for slight performance gains and more predictable behavior.

accessorKeys changed to remove console errors:

Warning: Encountered two children with the same key, roles. Keys should be unique so that components maintain their identity across updates. Non-unique keys may cause children to be duplicated and/or omitted — the behavior is unsupported and could change in a future version.

benhsm commented 1 year ago

These are good updates. One change before we merge this - could you update the role-related column defs to use accessor functions instead of accessor keys? Something like the following:

//...
    {
        header: 'lesson assistant',
        accessorFn: (row) => {
            const hasRole = row.roles.find(r => r.name == 'lessonAssistant')
            return hasRole ? 'Yes' : 'No'
        },
    },
//...

And you can delete any attributes you see there related to sizing, I was trying things and didn't mean to have those in the commit

parkerdavis1 commented 1 year ago

Sure, got it done.

Before I make the commit, just checking: Adding the accessorFn makes the cell function redundant, correct? As its currently constructed and styled, the div wrapper doesn't do anything. I could remove the extraneous divs. Not sure if you had plans for the divs that you hadn't gotten to yet.

benhsm commented 1 year ago

Yes, the accessorFn makes the cell function unnecessary, and we don't need the divs