icssc / AntAlmanac

A course exploration and scheduling tool for UCI Anteaters
https://antalmanac.com
MIT License
54 stars 64 forks source link

fix: colorpicker no longer closes on click #851

Closed KevinWu098 closed 7 months ago

KevinWu098 commented 7 months ago

Summary

  1. Bug was introduced by the refactor in #775
    updateEventsInCalendar = (close = true) => {
        this.setState({
            currentScheduleIndex: AppStore.getCurrentScheduleIndex(),
            eventsInCalendar: AppStore.getEventsInCalendar(),
            finalsEventsInCalendar: AppStore.getFinalEventsInCalendar(),
        });
        if (close) this.handleClosePopover();
    };

    ...

     componentDidMount = () => {
        AppStore.on('addedCoursesChange', this.updateEventsInCalendar);
        AppStore.on('customEventsChange', this.updateEventsInCalendar);
        AppStore.on('colorChange', this.updateEventsInCalendar);
        AppStore.on('currentScheduleIndexChange', this.updateEventsInCalendar);
        AppStore.on('scheduleNamesChange', this.updateScheduleNames);
    };

    componentWillUnmount = () => {
        AppStore.removeListener('addedCoursesChange', this.updateEventsInCalendar);
        AppStore.removeListener('customEventsChange', this.updateEventsInCalendar);
        AppStore.removeListener('colorChange', this.updateEventsInCalendar);
        AppStore.removeListener('currentScheduleIndexChange', this.updateEventsInCalendar);
        AppStore.removeListener('scheduleNamesChange', this.updateScheduleNames);
    };

Previously, handleClosePopover() would never be triggered as close always evaluated to false, even when no param was passed into updateEventsInCalendar. In the refactor, I omitted a check, since I expected nothing would change... haha

To whomever is reviewing this PR, if you can explain to me why that is the case when close = true in the snippet, please let me know.

Test Plan

  1. Color Picker shouldn't close on the Calendar Root when clicked.
  2. Popovers should close when events are changed and when index changes.

Issues

Bug Report: https://discord.com/channels/772739905981644850/780316580819107860/1192200165948395561