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

Fix display issue with TitledBox in Project Form #1658

Merged
merged 8 commits into from
Oct 15, 2024

Conversation

lcchrty
Copy link
Member

@lcchrty lcchrty commented Jun 4, 2024

Fixes #1646

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

  • Placed child components within TitleBox component to properly render the component
  • Bug introduced from previous PR

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

Visuals before changes are applied

image

Visuals after changes are applied

image

Copy link

github-actions bot commented Jun 4, 2024

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 lcchrty-lcchrty/issue1646 development
git pull https://github.com/lcchrty/VRMS.git lcchrty/issue1646

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.

In running this I'm getting an error. It looks like isLoading hasn't been defined or passed in as props. Can you take a look?

./src/components/ProjectForm.js
   Line 277:40:  'isLoading' is not defined  no-undef

@JackHaeg
Copy link
Member

@lcchrty just checking in! Would you be able to take a look at the error @spiteless posted above?

@lcchrty
Copy link
Member Author

lcchrty commented Jun 25, 2024

In running this I'm getting an error. It looks like isLoading hasn't been defined or passed in as props. Can you take a look?

./src/components/ProjectForm.js
   Line 277:40:  'isLoading' is not defined  no-undef

I'll look into this -- I think some edits from another issue I was working on wormed their way into this PR.

@trillium trillium self-requested a review July 15, 2024 19:25
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.

Sorry for slow-walking this. We ended up with a merge conflict.

Would you like to take a crack at it first? I'm happy to help resolve it if needed :)

# git fetch upstream
# git merge upstream/development
M  CONTRIBUTING.md
UU client/src/components/ProjectForm.js                   # merge conflict here
M  client/src/components/admin/donutChartContainer.js
M  client/src/components/admin/donutChartLoading.js
M  client/src/components/auth/Auth.js
M  client/src/pages/ManageProjects.js
M  client/src/sass/AdminLogin.scss

@lcchrty
Copy link
Member Author

lcchrty commented Jul 28, 2024

Hey @trilliumsmith, I found the problem! I was working on another ticket that got merged before this one, so there are missing pieces of state and some conditional rendering for the updated save button that isn't showing up. Screenshot below.
I'm planning to update this week before our first meeting in August!
Screenshot 2024-07-28 at 09 06 16

Fixes hackforla#1646
Merge updates to development branch and fixes merge conflicts
Copy link
Member Author

@lcchrty lcchrty left a comment

Choose a reason for hiding this comment

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

draft of requested changes

@lcchrty lcchrty requested a review from trillium September 17, 2024 02:28
@lcchrty lcchrty marked this pull request as draft September 24, 2024 03:50
@lcchrty lcchrty mentioned this pull request Sep 24, 2024
@lcchrty lcchrty marked this pull request as ready for review September 24, 2024 04:09
@lcchrty lcchrty mentioned this pull request Sep 24, 2024
@lcchrty
Copy link
Member Author

lcchrty commented Sep 24, 2024

@trillium merge conflict resolved! CI/CD failing though...

@JackHaeg
Copy link
Member

JackHaeg commented Oct 1, 2024

FYI - the failing tests are not an issue with your PR specifically, this has to do with the CI/CD pipeline.

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.

Thanks for this fix :)

@trillium trillium merged commit ec71fb3 into hackforla:development Oct 15, 2024
2 of 5 checks passed
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.

Fix display issue with TitledBox in Project Form
3 participants