devsoc-unsw / freerooms

A web application designed to aid UNSW students in finding vacant rooms.
https://freerooms.devsoc.app/
Other
19 stars 1 forks source link

463 - added previous day and next day arrows on mobile view #472

Closed azjy89 closed 4 months ago

azjy89 commented 6 months ago

also added some tests however they are not very extensive no comprehensive and may need to be improved upon.

azjy89 commented 5 months ago

In my new commits i have fixed the console warn issue image by turning on disableMaskedInput for the MUI date picker component.

Also rather than testing the containers I have tested the components which are seen by the viewer (namely the buttons and the Room Bookings) image as well as the calendar as a whole.

I changed to the best possible RTL queries using testing playground however some still can only use data-testid.

Mock data has been updated to better reflect real use.

Unnecessary imports have been removed.

The hover effect has been removed from the icon buttons to fit in better with the style of the rest of the page

JessicaF commented 5 months ago

Thank you for the changes !! 😎 I'll try see if I can find a way around the data-testid stuff/IconButton component (not sure if I can haha) but otherwise we'll merge you code in after I give it a go haha

JessicaF commented 4 months ago

Hi @azjy89, I've made some changes to your code - take a look and lmk what you think! I've updated BookingCalendar to use Date Picker v6 instead of v5 and combined it with other changes from the dev branch. I also changed the tests, in particular for checking if the correct icon buttons are shown in mobile view.

If you're happy with the changes and it works as you initially made it - then feel free to merge !!

azjy89 commented 4 months ago

Hi @JessicaF, I had a look at the made changes I think it looks good, everything works as intended, and everything makes sense to me now about the testing. Just about the disableRipple (below) on the IconButton do you want the disableRipple enabled to remove the circular onClick effect on the icon button or to leave the effect there? image

This is what it looks like disabled (below) image If this I will just merge.

and this is what it looks like enabled (no effect) image If this I'll make this change and then commit and then merge

Thanks for reviewing 👍

JessicaF commented 4 months ago

Hi @azjy89, I'm fine with having the ripple (i.e. not adding disableRipple) because on my side the buttons are no long circular but more square (fixed through having the borderRadius: 3:

Screenshot 2024-05-21 at 2 21 10 PM

I think it's fine to just merge in my final commit as long as everything else is okay!