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

Creating a new project form #1325

Merged
merged 21 commits into from
Mar 16, 2023

Conversation

MattPereira
Copy link
Contributor

@MattPereira MattPereira commented Mar 9, 2023

Fixes #1245

What changes did you make and why did you make them ?

  • Created ProjectForm.js that can hopefully be re-used for editing projects
  • Added a new route /projects/create that the "Add a New Project" button will take users to for creating a new project
  • Set most of the input styles in the theme.js file as I anticipate textfields will be the same everywhere in the app
  • Added all of the additional key value pairs necessary to create a project instead of just using project name like we used to
    • see the ProjectAPIService.js file
  • Set up the Save button to be disabled until all inputs are filled
  • Set up the Close button to redirect users to projects list page since the backend doesnt respond with necessary project id information that will be required to send a user to the project details page
  • Deleted the old component for creating new projects and the button that linked to it

Screenshots of Proposed Changes Of The Website (if any, please do not screen shot code changes)

Visuals before changes are applied

image

image

Visuals after changes are applied

image

image

Before Merging

@github-actions
Copy link

github-actions bot commented Mar 9, 2023

Want to review this pull request? Take a look at this documentation for a step by step guide!

From your project repository, check out a new branch and test the changes.

git checkout -b MattPereira-new-project-form development
git pull https://github.com/MattPereira/VRMS.git new-project-form

Copy link
Member

@trillium trillium left a comment

Choose a reason for hiding this comment

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

Great work!

Suggest add spacing for layout shift from helperText once we include form validation. Everything else looks good to me :)

client/src/App.js Show resolved Hide resolved
client/src/components/ProjectForm.js Show resolved Hide resolved
client/src/theme.js Show resolved Hide resolved
plang-psm

This comment was marked as outdated.

@MattPereira MattPereira added the draft Not ready for prioritization yet label Mar 10, 2023
@MattPereira

This comment was marked as outdated.

@MattPereira

This comment was marked as outdated.

@plang-psm

This comment was marked as outdated.

@MattPereira MattPereira removed the draft Not ready for prioritization yet label Mar 12, 2023
@MattPereira
Copy link
Contributor Author

MattPereira commented Mar 12, 2023

Stakeholder Testing Video: https://drive.google.com/file/d/1KBcMbdwVtjXTlIf3P6j0ecvHSc7i1vWt/view

Notes

  1. She wants location Remote to be the default
    • Figma still shows In-person as the default
  2. She is confused by GitHub Identifier field. Identifier is not necessary if you are already collecting GH URL
    • Figma shows Github Identifier field still existing
  3. She wants Slack Channel and explanation of "begins with"
    • Figma shows Slack URL input
  4. She says HFLA Website URLwould be hard to get during creation of project since theres no way hackforla.org will have a page for the project upon initial creation in VRMS
    • Figma shows HFLA Website URL input
  5. She wants to save project before being able to add recurring events
    • Figma shows Recurring Events component on the project creation page
    • From a development perspective, there is no possible way to add a recurring event to a project that does not exist yet
  6. Was not clear to user that Remote radio button is asking for a zoom link
    • How will user know which zoom link if there are multiple meetings?

Questions

  • Whose responsibility is it to update Figmas and issues based on what Bonnie says during testing?
  • Is Judy Sandbox the "source of truth" for figma designs that devs will implement?
  • Should alert be displayed after user clicks save? or just enable the close button and disable save? Should whole form be disabled after save? i.e. only option user has after saving is to click close
  • The Add a New Project button is secondary style in the figma, which for the save and close buttons is used to indicate that the button is disabled. Will this confuse user?

Copy link
Member

@plang-psm plang-psm left a comment

Choose a reason for hiding this comment

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

@MattPereira This is awesome! The form matches the Figma design and works great! I approve the changes but wanted to open up some discussion regarding the save/close buttons. I LOVE that you use the save button until all fields are complete, but I noticed that the user can't click the close button at this point. I know it is more likely that the user will click save more often than close but should we still give them the option to close as well?

Regardless, great job on this! 🚀

@MattPereira
Copy link
Contributor Author

@plang-psm I definitely welcome discussion of this edge case. I think we should ask for more clarification on the save and close buttons at the next meeting.

In the most recent testing video, the figma they are using shows the close button disabled when the save button is enabled so I'm going to leave it as is for now.

@MattPereira MattPereira merged commit fb95d9c into hackforla:development Mar 16, 2023
@trillium
Copy link
Member

Looking back through this:

// client/src/components/ProjectForm.js
// ...
<Typography sx={{ fontSize: '18px', fontWeight: '600' }}>
    Project Information
</Typography>
// ...

Should probaby have a varinat="h2"

<Typography varinat="h2" sx={{ fontSize: '18px', fontWeight: '600' }}>

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants