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

Refactor Select Coaching Session Component #37

Draft
wants to merge 13 commits into
base: main
Choose a base branch
from
Draft

Conversation

jhodapp
Copy link
Member

@jhodapp jhodapp commented Oct 5, 2024

Description

This PR adds the useSWR hook from the swr library as a wrapper around a global fetcher hook (useApiData). It allows any component to access the data it needs for rendering dynamic values from the API.

A dashboard component is refactored to use the new hook to fetch data for conditionally rendered select components and a button. The JoinCoachingSession component allows a user to select an organization, a coaching relationship and a session and join the session with a button click.

Changes

  • Adds a hook for fetching data from the API
  • Adds a Dynamic select component that renders dropdowns with data fetched from the API
  • Renders 3 dynamic select components in a composite or ‘smart’ JoinCoachingSession component responsible for setting organization, relationship and session IDs in the state tree.
  • Merged with Jim's upstream changes

Screenshots / Videos Showing UI Changes (if applicable)

Screenshot 2024-10-03 at 3 12 08 PM

Testing Strategy

Login and attempt to join a session using the component illustrated in the image

Concerns

  • The rendering helper functions in the dynamic-api-select component should be separated into its own component.
  • PRs of this kind should be broken up into smaller features in future to improve code merge 😅

@jhodapp jhodapp added the enhancement Improves existing functionality or feature label Oct 5, 2024
@jhodapp jhodapp self-assigned this Oct 5, 2024
@jhodapp jhodapp requested a review from qafui October 5, 2024 21:50
…hing else in the application relies on "objectId"
const TO_DATE = DateTime.now().plus({ month: 1 }).toISODate();

const handleOrganizationSelection = (id: Id) => {
fetchOrganization(id)
Copy link
Member Author

Choose a reason for hiding this comment

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

@qafui This is what I'm referring to, can we/should we rework fetchOrganization to use your use-api-data in this PR, or do that across the board in a second fast-follower PR? I'm open to either option.

};

const handleRelationshipSelection = (id: Id) => {
fetchCoachingRelationshipWithUserNames(organization.id, id)
Copy link
Member Author

Choose a reason for hiding this comment

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

@qafui Same thing here...

};

const handleSessionSelection = (id: Id) => {
fetchCoachingSessions(relationship.id)
Copy link
Member Author

Choose a reason for hiding this comment

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

@qafui and here again...

@@ -20,7 +20,7 @@ interface AppState {
coachingRelationship: CoachingRelationshipWithUserNames;
}

interface AppStateActions {
export interface AppStateActions {
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
export interface AppStateActions {
interface AppStateActions {

@qafui this is meant to be private to AppStateStore, any reason to broaden its scope?

@jhodapp jhodapp marked this pull request as draft October 15, 2024 17:50
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improves existing functionality or feature
Projects
Status: Review
Development

Successfully merging this pull request may close these issues.

2 participants