fadwamahmoud
commented
8 months ago All in all, this seems to do what it's supposed to. I find it counterproductive for me to spend some significant amount of time going through the code. Here are some soft notes (you can chose to fix them up or not, it's up to you, all in all I wouldn't spend too much time on them, especially if the customer is happy):
* I'm a bit confused with the colour coding - I understand green is where the booked/attended match and red is where they don't (but only on ice slots), whereas yellow is where they don't match on off ice...but: **if it's clear for the user, I'm good** * hover: I would make the entire table have `select-none` class and would make it so that diff appears when you hover over the table cell (not the text inside the cell) * diff: Having `1` for "attended 1 hour more than booked" and `-1` for one hour less than booked is mathematically correct, but in terms of clearer communication I would include the `+` sign: * `+1` attended 1 hour more * `-1` attended 1 hour less * Is the colour coding of the rows (blue/yellow) enough for the admin to distinguish between ice and off ice? (I'm guessing it is and this was at their request)All in all, this can go in AFAIC
I think your comments are valid and yes the color code made me itch a bit but Silvio approved it and they don't seem to be concerned about much but the table being compact so I'd rather not work on this any longer until they use it and wait see if they want things differently.
Fixes #896