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

Added renaming year on roadmap #368

Merged
merged 7 commits into from
Jan 19, 2024
Merged

Added renaming year on roadmap #368

merged 7 commits into from
Jan 19, 2024

Conversation

kylerpan
Copy link
Contributor

@kylerpan kylerpan commented Oct 19, 2023

Issue #300

Description

  • Created an extra form group on top of 'Start Year' to change the name (within 'Edit Year')
    • Both for edit year and add year
  • Added 'name' attribute to PlannerData to refer to the name of the year
  • Added new reducer to edit PlannerData 'name' attribute
  • Trimmed the name users enter for duplicate checking purposes

Potential concerns

  • No check if the name is empty
  • The 'Clear' button doesn't reset the name

Screenshots

Screen.Recording.2023-10-18.at.5.24.58.PM.mov

Steps to verify/test this change:

  • Verify changes work as expected on staging instance

Final Checks:

  • Verify successful deployment
  • Delete branch

(optional)

  • Write tests
  • Write documentation

Issues

Closes #300

@kylerpan kylerpan requested a review from js0mmer October 19, 2023 00:26
Copy link
Member

@js0mmer js0mmer left a comment

Choose a reason for hiding this comment

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

Nice work. I do agree that being able to set no name is a concern. I think there should be some input constraints: non-empty and also a character limit.
I'm not sure what you mean about the clear button, it seems work fine for me. Also requesting an additional review from @MFYLM since he's worked with the planner page.

site/src/pages/RoadmapPage/Year.tsx Outdated Show resolved Hide resolved
@js0mmer js0mmer requested a review from MFYLM October 19, 2023 01:44
@kylerpan kylerpan self-assigned this Oct 20, 2023
site/src/pages/RoadmapPage/AddYearPopup.tsx Outdated Show resolved Hide resolved
Copy link

Deployed staging instance to https://staging-368.peterportal.org

@js0mmer js0mmer merged commit b3574f0 into master Jan 19, 2024
2 checks passed
@js0mmer js0mmer deleted the kyle/allow-naming-years branch January 19, 2024 02:20
@js0mmer js0mmer mentioned this pull request Feb 10, 2024
2 tasks
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.

Allow Naming Years in Peter's Roadmap
2 participants