Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix mobile location tab #1106

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 12 additions & 10 deletions apps/antalmanac/src/components/Calendar/CourseCalendarEvent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,15 @@ import { Chip, IconButton, Paper, Tooltip, Button } from '@material-ui/core';
import { Theme, withStyles } from '@material-ui/core/styles';
import { ClassNameMap, Styles } from '@material-ui/core/styles/withStyles';
import { Delete, Search } from '@material-ui/icons';
import { useMediaQuery } from '@mui/material';
import { WebsocSectionFinalExam } from '@packages/antalmanac-types';
import { useEffect, useRef, useCallback } from 'react';
import { Event } from 'react-big-calendar';
import { Link } from 'react-router-dom';

import MapLink from '../../components/buttons/MapLink';
import { MOBILE_BREAKPOINT } from '../../globals';

import { deleteCourse, deleteCustomEvent } from '$actions/AppStoreActions';
import CustomEventDialog from '$components/Calendar/Toolbar/CustomEventDialog/';
import ColorPicker from '$components/ColorPicker';
Expand Down Expand Up @@ -169,9 +173,9 @@ const CourseCalendarEvent = (props: CourseCalendarEventProps) => {

const { isMilitaryTime } = useTimeFormatStore();
const isDark = useThemeStore((store) => store.isDark);

const isMobileScreen = useMediaQuery(`(max-width: ${MOBILE_BREAKPOINT})`);
const focusMap = useCallback(() => {
setActiveTab(2);
setActiveTab(isMobileScreen ? 3 : 2);
}, [setActiveTab]);

const { classes, selectedEvent } = props;
Expand Down Expand Up @@ -262,14 +266,12 @@ const CourseCalendarEvent = (props: CourseCalendarEventProps) => {
<td className={`${classes.multiline} ${classes.rightCells}`}>
{locations.map((location) => (
<div key={`${sectionCode} @ ${location.building} ${location.room}`}>
<Link
className={classes.clickableLocation}
to={`/map?location=${locationIds[location.building] ?? 0}`}
onClick={focusMap}
color={isDark ? '#1cbeff' : 'blue'}
>
{location.building} {location.room}
</Link>
<MapLink
buildingId={locationIds[location.building] ?? '0'}
buildingName={`${location.building} ${location.room}`}
focusMap={focusMap}
isDark={isDark}
/>
</div>
))}
</td>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,10 @@
import { Box } from '@mui/material';
import { useMediaQuery } from '@mui/material';
import { WebsocSectionMeeting } from '@packages/antalmanac-types';
import { Fragment, useCallback } from 'react';
import { Link } from 'react-router-dom';

import { MOBILE_BREAKPOINT } from '../../../../../globals';
import MapLink from '../../../../buttons/MapLink';

import { TableBodyCellContainer } from '$components/RightPane/SectionTable/SectionTableBody/SectionTableBodyCells/TableBodyCellContainer';
import locationIds from '$lib/location_ids';
Expand All @@ -16,9 +19,10 @@ interface LocationsCellProps {
export const LocationsCell = ({ meetings }: LocationsCellProps) => {
const isDark = useThemeStore((store) => store.isDark);
const { setActiveTab } = useTabStore();
const isMobileScreen = useMediaQuery(`(max-width: ${MOBILE_BREAKPOINT})`);

const focusMap = useCallback(() => {
setActiveTab(2);
setActiveTab(isMobileScreen ? 3 : 2);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe you need to add the isMobileScreen to the useCallback's dependency array to allow this fix to work when the screen is resized.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think in putting the isMobileScreen to the useCallback's dependency array causes the whole page to want to refresh for some reason, while putting it before doesn't.

}, [setActiveTab]);

return (
Expand All @@ -30,16 +34,12 @@ export const LocationsCell = ({ meetings }: LocationsCellProps) => {
const buildingId = locationIds[buildingName];
return (
<Fragment key={meeting.timeIsTBA + bldg}>
<Link
style={{
textDecoration: 'none',
}}
to={`/map?location=${buildingId}`}
onClick={focusMap}
color={isDark ? 'dodgerblue' : 'blue'}
>
{bldg}
</Link>
<MapLink
buildingId={buildingId}
buildingName={bldg}
focusMap={focusMap}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The point to the MapLink is to move the map-focusing business logic into there so that we don't have to keep re-writing the focusMap component.

isDark={isDark}
/>
<br />
</Fragment>
);
Expand Down
26 changes: 26 additions & 0 deletions apps/antalmanac/src/components/buttons/MapLink.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import React from 'react';
import { Link } from 'react-router-dom';

interface MapLinkProps {
buildingId: number;
buildingName: string;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a misnomer, since it has both the building and the room number

focusMap: () => void;
isDark: boolean;
}

const MapLink: React.FC<MapLinkProps> = ({ buildingId, buildingName, focusMap, isDark }) => {
return (
<Link
to={`/map?location=${buildingId}`}
onClick={focusMap}
style={{
textDecoration: 'none',
color: isDark ? 'dodgerblue' : 'blue',
}}
>
{buildingName}
</Link>
);
};

export default MapLink;