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

Add star button to activity description #132

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

snowmonkey18
Copy link
Contributor

Adds star button to ActivityDescription, since someone requested to be able to star a class when its ActivityDescription is up, without having to search for it on the ClassTable.

snowmonkey18 and others added 14 commits December 30, 2024 23:21
This avoids the nesting indentation that comes from multiple sequential
requests.
This resolves one of the TODO comments, but it is useful since it means
that the URL in the address bar matches what is shown on the page.
IAP or summer of a year where those terms were not scraped separately
should redirect silently to spring or fall of that year, and otherwise
we try to redirect (eventually with some sort of notice to the user) to
the same semester of the current year.
Copy link
Member

@cjquines cjquines left a comment

Choose a reason for hiding this comment

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

i would kinda almost prefer it to not be in classtable but i'd leave that to others

Comment on lines +420 to +427
const refreshCells = () => {
gridRef.current?.api?.refreshCells({
force: true,
columns: ["number"],
});
};
window.addEventListener("refreshStarCells", refreshCells);
return () => window.removeEventListener("refreshStarCells", refreshCells);
Copy link
Member

Choose a reason for hiding this comment

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

thought: this is very cursed but i'm not sure what else can be done about it

maybe just add a comment on both sides...

Copy link
Collaborator

Choose a reason for hiding this comment

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

Maybe we could pass gridRef up into App and back down into ClassTable?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants