Skip to content

Conversation

@TheSKBroook
Copy link
Contributor

@TheSKBroook TheSKBroook commented Sep 29, 2025

Description

  • Make a generic component of CreateSemesterPopUpFlow in a flow like this :
    • Semester Name Pop Up
    • Semester Date Pop Up
    • Semester Date Pop Up -- for break
    • Semester Create Pop UP
  • Make StoryBook for every component
  • Make tests for every component
image image image image image
Flow.Recording.mp4

Fixes #805 (issue)

Type of change

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

How Has This Been Tested?

  • Manual testing (requires screenshots or videos)
  • Integration tests written (requires checks to pass)

Checklist before requesting a review

  • My code follows the style guidelines of this project
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • I have added thorough tests that prove my fix is effective and that my feature works
  • I've requested a review from another user

@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2025

Preview Deployment Status

✅ Storybook Preview: https://3a40ef8b.uabc-storybook.pages.dev

✅ Frontend Preview: https://ce7a7a82.uabc.pages.dev

@github-actions
Copy link
Contributor

github-actions bot commented Sep 29, 2025

Coverage Report

Status Category Percentage Covered / Total
🟢 Lines 84.58% (🎯 70%)
⬇️ -0.30%
2441 / 2886
🟢 Statements 84.51% (🎯 70%)
⬇️ -0.31%
2505 / 2964
🟢 Functions 81.08% (🎯 80%)
⬇️ -0.28%
587 / 724
🟢 Branches 78.34% (🎯 60%)
⬇️ -0.17%
1338 / 1708
File Coverage
File Stmts Branches Functions Lines Uncovered Lines
Changed Files
packages/ui/src/components/Generic/CreateSemesterPopUpFlow/CreateSemesterPopUpFlow.tsx 65.71% 72.22% 50% 64.7% 36-39, 46-54, 58, 105-106, 110-111, 115, 124-131
packages/ui/src/components/Generic/CreateSemesterPopUpFlow/SemesterCreatedPopUp/SemesterCreatedPopUp.tsx 100% 100% 100% 100%
packages/ui/src/components/Generic/CreateSemesterPopUpFlow/SemesterDatePopUp/SemesterDatePopUp.tsx 75% 75% 83.33% 76.66% 103, 117, 123-131
packages/ui/src/components/Generic/CreateSemesterPopUpFlow/SemesterNamePopUp/SemesterNamePopUp.tsx 100% 100% 100% 100%
Generated in workflow #3627 for commit 83175f3 by the Vitest Coverage Report Action

@TheSKBroook TheSKBroook marked this pull request as ready for review September 29, 2025 16:14
@TheSKBroook TheSKBroook requested a review from Copilot September 29, 2025 16:14
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR implements a multi-step semester creation flow with four modal dialogs that collect semester information through a guided process. The flow progresses from semester naming to date selection for both the semester period and breaks, concluding with a success confirmation.

Key changes include:

  • Implementation of four reusable popup components with form validation and user interaction handling
  • A main orchestrator component that manages the entire flow state and step progression
  • Comprehensive test coverage and Storybook documentation for all components

Reviewed Changes

Copilot reviewed 19 out of 19 changed files in this pull request and generated 2 comments.

File Description
packages/ui/src/components/Generic/index.ts Exports the new CreateSemesterPopUpFlow component
packages/ui/src/components/Generic/CreateSemesterPopUpFlow/* Core flow component and individual step components with tests and stories
packages/shared/src/types/semester.ts Type definitions for semester popup form values
packages/shared/src/schemas/semester.ts Zod validation schemas for semester popup forms

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

open={open && state.step === 2}
semesterName={state.semesterName || "Semester"}
subtitle={`Select the start and end dates for ${state.semesterName || "the semester"} break period`}
title={"Semester Break" + "\n" + "Start & End"}
Copy link

Copilot AI Sep 29, 2025

Choose a reason for hiding this comment

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

String concatenation with newline characters should use template literals for better readability. Change to title={\Semester Break\nStart & End`}`.

Suggested change
title={"Semester Break" + "\n" + "Start & End"}
title={`Semester Break\nStart & End`}

Copilot uses AI. Check for mistakes.
Copy link
Contributor Author

@TheSKBroook TheSKBroook Sep 29, 2025

Choose a reason for hiding this comment

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

I tried this, title={Semester Break\nStart & End}, but when I committed, it says that I should not add curly braces when there is no JFX.

JSX attribute value does not need to be wrapped in curly braces.

Copy link
Member

Choose a reason for hiding this comment

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

Have you tried

title="Semester Break\nStart & End"

Copy link
Contributor Author

@TheSKBroook TheSKBroook Oct 1, 2025

Choose a reason for hiding this comment

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

Try it but it still shows the literal text.

I'll do
{title.replace(/\\n/g, '\n')} in the components and so we could just do this
title="Semester Break\nStart & End"

TheSKBroook and others added 2 commits September 30, 2025 05:16
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@TheSKBroook TheSKBroook requested a review from Copilot October 1, 2025 10:25
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

Copilot reviewed 19 out of 19 changed files in this pull request and generated 3 comments.


Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

textAlign="center"
whiteSpace="pre-line"
>
{title.replace(/\\n/g, "\n")}
Copy link

Copilot AI Oct 1, 2025

Choose a reason for hiding this comment

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

The string replacement logic title.replace(/\\n/g, \"\n\") should be extracted to a utility function or handled more elegantly. Consider using a proper multiline string approach or a dedicated text formatting utility.

Copilot uses AI. Check for mistakes.
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.

[FRONTEND] Create CreateSemesterFlow generic component

3 participants