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(research): refactoring upload submit modals to remove some duplication #3518

Closed

Conversation

codisart
Copy link
Contributor

@codisart codisart commented May 9, 2024

PR Checklist

PR Type

  • 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 change)
  • Developer experience (improves developer workflows for contributing to the project)

Description

What this PR does

Breakdown the upload status modal into several smaller components to remove duplication and to allow better readibility


What happens next?

Thanks for the contribution! We try to make sure all PRs are reviewed ahead of our monthly maintainers call (first Monday of the month)

If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.

If you need more immediate feedback you can try reaching out on Discord in the Community Platform development channel.

@codisart codisart requested a review from a team as a code owner May 9, 2024 13:23
@codisart codisart changed the title refacto(research): refactoring upload submit modals to remove some du… refacto(research): refactoring upload submit modals to remove some duplication May 9, 2024
@codisart codisart force-pushed the refacto/research-submit-modal branch from e7d8cf4 to a0c3452 Compare May 9, 2024 13:45
@codisart codisart changed the title refacto(research): refactoring upload submit modals to remove some duplication refactor(research): refactoring upload submit modals to remove some duplication May 9, 2024
@codisart codisart force-pushed the refacto/research-submit-modal branch 2 times, most recently from 7b4d0fc to d41b1b1 Compare May 9, 2024 19:21
@benfurber
Copy link
Member

Hey @codisart, I can see the unit test is failing.

Another thought. There's probably some duplication with the HowToSubmitStatus. Could a SubmitStatus component be created they could both have as a parent? Similar to either the layout/styling only and/or store access in #3356.

@benfurber benfurber added the 🤝 Awaiting author Waiting on action from the author label May 15, 2024
@codisart
Copy link
Contributor Author

Hey @codisart, I can see the unit test is failing.

Another thought. There's probably some duplication with the HowToSubmitStatus. Could a SubmitStatus component be created they could both have as a parent? Similar to either the layout/styling only and/or store access in #3356.

Yes you right. Where I should put the common component ? In the packages/components folder ?

@benfurber
Copy link
Member

benfurber commented May 21, 2024

Hey @codisart, I can see the unit test is failing.
Another thought. There's probably some duplication with the HowToSubmitStatus. Could a SubmitStatus component be created they could both have as a parent? Similar to either the layout/styling only and/or store access in #3356.

Yes you right. Where I should put the common component ? In the packages/components folder ?

Both? :) At least one. For design/layout stuff, aim for the components library. Stuff that's linked to the stores, etc. for common.

Looking at the specifics here. I'd say component library. There's a command in the readme for generating a new component with a story and test files.

@benfurber benfurber force-pushed the refacto/research-submit-modal branch from bfa6aa2 to 5516cd0 Compare June 24, 2024 14:03
@benfurber
Copy link
Member

Spend a bit of time trying to get this to work for me. Unfortunately after a bit of time I can't get the modal to appear for me when I click submit/save. I'm going to close this for the moment. Very happy for you to pick this up again in the future @codisart if you get any time.

@benfurber benfurber closed this Jun 24, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

2 participants