-
Notifications
You must be signed in to change notification settings - Fork 2
Refactor Dashboard page - remove org and rel selectors and enhance sessions list #104
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
Refactor Dashboard page - remove org and rel selectors and enhance sessions list #104
Conversation
…age. Moves CoachingSessionList left. Adds a tab bar splitting sessions out by upcoming/previous sessions.
…or the layout of this component on the Dashboard page, and make logout more robust.
…rer and nicer to read.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jhodapp looks great! I left some thoughts but I don't feel strong enough about any of it to request changes. Curious on your thoughts though.
src/middleware.ts
Outdated
@@ -19,18 +19,18 @@ export default async function middleware(req: NextRequest) { | |||
|
|||
// 3. Get the session from the cookie | |||
const sessionCookie = req.cookies.get("id"); | |||
const session = sessionCookie?.value; | |||
const validSession = sessionCookie?.value; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if isValidSession
would make sense here? Especially since we're treating it as a boolean in, from what I can tell, every use.
The main reason I bring it up is that it feels somewhat misleading to call it validSession
when it actually could represent a not valid session.
or if you wanted to be more explicit you could do something maybe like:
const validSession = sessionCookie?.value; | |
const isValidSession = !!sessionCookie?.value; |
That way there is not ambiguity.
Probably not a huge deal either way. I don't consider it a blocker
src/types/coaching-session.ts
Outdated
} | ||
|
||
// export function sortCoachingSessionArray( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this removable?
Description
This PR refactors the Dashboard page removing unnecessary components and improving the CoachingSessionList component to display upcoming/previous sessions in two different tabs.
It also improves the logout robustness, but a separate PR will be necessary to fully address #101
Notes
GitHub Issue: Closes #87
Changes
Screenshots / Videos Showing UI Changes (if applicable)
No relationship selected:

Relationship selected, upcoming sessions:

Relationship selected, previous sessions:

Testing Strategy
Concerns
None