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

Set Up Container Component for MTV Checkout Workflows #13334

Merged
merged 12 commits into from
Feb 6, 2020

Conversation

jcq
Copy link
Contributor

@jcq jcq commented Feb 3, 2020

Connects #13318

Description

This does multiple things:

  • refactors QueueApp to break out MTV routes into their own file
  • creates MotionToVacateContext for data handling as part of MTV checkout flow
  • creates MotionToVacateFlowContainer that will serve as a container for the MTV checkout flows

Acceptance Criteria

  • Code compiles correctly

@codeclimate
Copy link

codeclimate bot commented Feb 3, 2020

Code Climate has analyzed commit beced6a and detected 0 issues on this pull request.

View more on Code Climate.

JC Quirin added 3 commits February 3, 2020 16:32
(needed to use default export);
commented out now-unused MTV-related items from QueueApp;
Copy link
Contributor

@leikkisa leikkisa left a comment

Choose a reason for hiding this comment

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

I'd prefer some of the stuff that is getting taken out to be removed altogether instead of commented out, for being merged into master. This is so WIP-y (although TBH the feature itself is still a WIP!).

I am also still feeling a little bit iffy about the extra piece of the URL, I might need you to explain more about how it's being used.

client/app/queue/mtv/MotionToVacateFlowContainer.jsx Outdated Show resolved Hide resolved
client/app/queue/QueueApp.jsx Outdated Show resolved Hide resolved
client/app/queue/mtv/MotionToVacateFlowContainer.jsx Outdated Show resolved Hide resolved
client/app/queue/mtv/motionToVacateRoutes.js Show resolved Hide resolved
Copy link
Contributor

@leikkisa leikkisa left a comment

Choose a reason for hiding this comment

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

This is very nicely done 👍 🏆 . I think this approach will require some modifications to the task behavior which I'm thinking is going to be in part of the other PR, but I like this set up a lot in the meantime.

@@ -461,6 +434,8 @@ class QueueApp extends React.PureComponent {
title="User Management | Caseflow"
render={this.routedUserManagement}
/>

{motionToVacateRoutes.page}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is very pretty and nicely done!

@@ -657,6 +618,8 @@ class QueueApp extends React.PureComponent {
<Route path="/team_management/add_vso" render={this.routedAddVsoModal} />
<Route path="/team_management/add_private_bar" render={this.routedAddPrivateBarModal} />
<Route path="/team_management/lookup_participant_id" render={this.routedLookupParticipantIdModal} />

{motionToVacateRoutes.modal}
Copy link
Contributor

Choose a reason for hiding this comment

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

@jcq jcq added the Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master label Feb 6, 2020
@va-bot va-bot merged commit 5239ba2 into master Feb 6, 2020
@va-bot va-bot deleted the jc/13318-mtv-container branch February 6, 2020 23:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ready-to-Merge This PR is ready to be merged and will be picked up by va-bot to automatically merge to master
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants