Ap/no notifications husky page #265

ananyaspatil · 2024-04-14T05:57:48Z

Purpose

Creates an EmptyCard to display for the Empty Notifications page.
When "Search for classes" button is hovered over, crying-husky image changes to happy-husky image.
Includes a placeholder text for "Spring 2023 Notifications", this will need to be changed to actually reflect the semester of notifications being displayed.

Tickets

https://trello.com/c/SoJpDQFG

Contributors

@ananyaspatil @Anzhuo-W

Feature List

Created display for the Empty Notifications page.

Created EmptyCard Component: contains display text and a button that redirects to the default search page
added crying-husky.svg and happy-husky.svg images
When "Search for classes" button is hovered over, crying-husky image changes to happy-husky image.
Added isHovering const to EmptyCard
Added logic to update husky image to happy when OnMouseEnter is true for "search for classes" button
Heading text updates to reflect the semester of notifications being displayed (should always be the most recent semester).

Pictures

No Hover:

Hover:

Reviewers

Primary reviewer:
@pranavphadke1 @sebwittr

Secondary reviewers:
@soulwa

vercel · 2024-04-14T05:57:55Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
searchneu	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Dec 4, 2024 3:40am

ananyaspatil · 2024-04-14T06:00:10Z

components/SubscriptionsPage/EmptyCard.tsx

+                    onClick={() => {
+                      router.push('NEU');
+                    }}
+                    onMouseEnter={() => setIsHovering(true)}


To change image upon hovering, am setting isHovering constant to true when mouse enters the Button and to false when mouse leaves the button.

ananyaspatil · 2024-04-14T06:00:58Z

components/SubscriptionsPage/EmptyCard.tsx

+
+export const EmptyCard = (): ReactElement => {
+  const router = useRouter();
+  const [isHovering, setIsHovering] = useState(false);


Added const isHovering so that Husky image can be changed when mouse hovers over the "search for classes" button

ananyaspatil · 2024-04-14T06:02:08Z

pages/subscriptions.tsx

@@ -131,6 +135,10 @@ export default function SubscriptionsPage(): ReactElement {
        await fetchSectionNotifs();
        setClasses(classMapping);
        setIsFetching(false);
+        // are there classes the user is subscribed to?


Adding condition for when to display the No Notifications page

sebwittr

This looks good, main thing is the changes to ClassCardWrapper. I think we shouldn't need to touch that file at all, since it's only supposed to wrap actual class cards. The EmptyCard should serve as its own component, and not really need to rely on that.

sebwittr · 2024-04-14T16:13:33Z

components/SubscriptionsPage/ClassCard.tsx

-        <div className="SearchResult__header--left">{headerLeft}</div>
-        {headerRight}
-      </div>
+      <div>{headerLeft}</div>


I think we needed the styles from SearchResult__header and SearchResult__header--left for the class cards. Did your changes actually need to change this file?

sebwittr · 2024-04-14T16:14:49Z

components/SubscriptionsPage/EmptyCard.tsx

+
+  return (
+    <>
+      <div className="Results_Container">


We probably don't need all these extra styles, and it would be better to make a separate style sheet for EmptyCards.

sebwittr · 2024-04-14T16:16:08Z

styles/pages/_Results.scss

+      padding-bottom: 0px;
+    }
+
+    .SearchResult {


I would move these out into a separate _EmptyCard.scss so we don't put too much into this file.

Anzhuo-W · 2024-11-17T21:52:10Z

Trello ticket

…-husky-page

…ieval of Term name.

Properly migrate styles to new _EmptyCard file

cherman23 · 2024-11-21T17:34:43Z

Styles have been moved to new EmptyStyles, Empty Card automatically updates to show current semester.

mehallhm

Generally looks pretty good, just some comments on code consistency and readability for the rest of the codebase

mehallhm · 2024-11-22T14:28:04Z

components/SubscriptionsPage/ClassCard.tsx

@@ -13,18 +13,17 @@ type ClassCardWrapperType = {
  afterBody?: ReactElement;
 };

-const ClassCardWrapper = ({
+export const ClassCardWrapper = ({


Let's match the rest of the codebase here and use function definitions rather than arrow notations. It seems to be like a 70 / 30 split on what we do, but we should pick one to stick with, and I'll just go with the majority

addressed in recent commit

mehallhm · 2024-11-22T14:30:11Z

package.json

@@ -43,6 +43,7 @@
    "js-yaml-loader": "^1.2.2",
    "jsonwebtoken": "^8.5.1",
    "jwt-decode": "^3.1.2",
+    "leading-trim": "^1.0.2",


I would like to see evidence that adding a package is the correct call here. It's fine if we actually need it, but I would like that proof

addressed in recent commit

mehallhm · 2024-11-22T14:34:11Z

pages/subscriptions.tsx

@@ -133,6 +137,10 @@ export default function SubscriptionsPage(): ReactElement {
        await fetchSectionNotifs();
        setClasses(classMapping);
        setIsFetching(false);
+        // are there classes the user is subscribed to?
+        if (classMapping.size > 0) {
+          setIsSubscribed(true);


Why is a state variable being reassigned to another state variable? I would just use classMapping.size > 0 instead of managing a derived piece of state

we will address this in a future ticket

mehallhm · 2024-11-22T14:36:20Z

pages/subscriptions.tsx

@@ -158,25 +166,33 @@ export default function SubscriptionsPage(): ReactElement {
      {isFetching ? (


I don't love the ternary, especially nested ones. Would an early return be better? This is really not that important but just thinking from a readability standpoint.

we will address this in a future ticket

- fix styles in classcard, use function notation - Redirect Empty card to empty results page.

…andboxnu/searchneu into AP/no-notifications-husky-page

This reverts commit 0dad192.

Anzhuo-W and others added 6 commits March 30, 2024 03:01

Add empty subscriptions logic

0df3953

Create EmptyCard component

948c3c9

Add styling and assets

67d82ee

Update styling of EmptyCard

1ac9258

Modified styling, added on hover image change functionality

0f1d91f

edited line-heights

1834cae

ananyaspatil requested review from sebwittr and pranavphadke1 April 14, 2024 05:57

ananyaspatil commented Apr 14, 2024

View reviewed changes

ananyaspatil assigned ananyaspatil and Anzhuo-W Apr 14, 2024

ananyaspatil marked this pull request as ready for review April 14, 2024 06:04

resolve merge conflicts

d869b35

vercel bot deployed to Preview April 14, 2024 06:09 View deployment

resolved sass-lint errors

cd6e8c3

vercel bot had a problem deploying to Preview April 14, 2024 06:30 Failure

actually resolved sass-lint errors

9f91b75

vercel bot had a problem deploying to Preview April 14, 2024 06:35 Failure

really actually resolved sass-lint errors this time

1fca833

vercel bot deployed to Preview April 14, 2024 07:07 View deployment

sebwittr requested changes Apr 14, 2024

View reviewed changes

cherman23 added 4 commits November 20, 2024 20:54

Update Empyt Card to include term data.

bec72d1

Merge remote-tracking branch 'origin/master' into AP/no-notifications…

bec9fbd

…-husky-page

Move styles from _Results to new _EmptyCard to declutter.

36c9931

Remove empty card styles from _Results

f8a61b5

vercel bot had a problem deploying to Preview November 21, 2024 02:50 Failure

Revert styles, fix empty card props. subscriptions page requires retr…

9d93283

…ieval of Term name.

vercel bot deployed to Preview November 21, 2024 05:20 View deployment

Empty Card Semester Name updates automatically

86f8b55

Properly migrate styles to new _EmptyCard file

vercel bot deployed to Preview November 21, 2024 17:22 View deployment

mehallhm reviewed Nov 22, 2024

View reviewed changes

mehallhm assigned cherman23 Nov 24, 2024

- remove unused leading-trim module

be2ac4a

- fix styles in classcard, use function notation - Redirect Empty card to empty results page.

vercel bot had a problem deploying to Preview November 24, 2024 21:06 Failure

remove default from ClassCard export

a10405a

vercel bot deployed to Preview November 24, 2024 21:11 View deployment

mehallhm added 2 commits November 27, 2024 12:01

Add unsubscribe button functionality

a2e81f2

Notif course fetching refactor

811f9a6

vercel bot deployed to Preview November 27, 2024 20:04 View deployment

mehallhm added 2 commits November 27, 2024 13:04

Remove local config

68e6802

Remove yarn install state

59c37ef

vercel bot deployed to Preview November 27, 2024 20:06 View deployment

cherman23 added 2 commits December 3, 2024 22:24

Add pill to subscription class card

77461c7

Merge branch 'AP/no-notifications-husky-page' of https://github.com/s…

7ebacba

…andboxnu/searchneu into AP/no-notifications-husky-page

vercel bot deployed to Preview December 4, 2024 03:35 View deployment

fix lint colors issue

c0de88f

vercel bot deployed to Preview December 4, 2024 03:40 View deployment

ananyaspatil merged commit 0dad192 into master Dec 4, 2024
5 checks passed

ananyaspatil deleted the AP/no-notifications-husky-page branch December 4, 2024 22:48

ananyaspatil added a commit that referenced this pull request Dec 4, 2024

Revert "Ap/no notifications husky page (#265)"

1a2af61

This reverts commit 0dad192.

ananyaspatil mentioned this pull request Dec 4, 2024

Revert "Ap/no notifications husky page" #302

Merged

ananyaspatil added a commit that referenced this pull request Dec 4, 2024

Revert "Ap/no notifications husky page (#265)" (#302)

c70a998

This reverts commit 0dad192.

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Ap/no notifications husky page #265

Ap/no notifications husky page #265

ananyaspatil commented Apr 14, 2024 •

edited

Loading

vercel bot commented Apr 14, 2024 •

edited

Loading

ananyaspatil Apr 14, 2024

ananyaspatil Apr 14, 2024

ananyaspatil Apr 14, 2024

sebwittr left a comment

sebwittr Apr 14, 2024

sebwittr Apr 14, 2024

sebwittr Apr 14, 2024

Anzhuo-W commented Nov 17, 2024

cherman23 commented Nov 21, 2024

mehallhm left a comment

mehallhm Nov 22, 2024

cherman23 Nov 24, 2024

mehallhm Nov 22, 2024

cherman23 Nov 24, 2024

mehallhm Nov 22, 2024

cherman23 Nov 24, 2024

mehallhm Nov 22, 2024

cherman23 Nov 24, 2024

		@@ -158,25 +166,33 @@ export default function SubscriptionsPage(): ReactElement {
		{isFetching ? (

Ap/no notifications husky page #265

Ap/no notifications husky page #265

Conversation

ananyaspatil commented Apr 14, 2024 • edited Loading

Purpose

Tickets

Contributors

Feature List

Pictures

Reviewers

vercel bot commented Apr 14, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sebwittr left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Anzhuo-W commented Nov 17, 2024

cherman23 commented Nov 21, 2024

mehallhm left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ananyaspatil commented Apr 14, 2024 •

edited

Loading

vercel bot commented Apr 14, 2024 •

edited

Loading