Move all modals into ModalRouter and independent modal components #637

Closed
opened 2017-09-29 22:12:57 +02:00 by kauffj · 5 comments
kauffj commented 2017-09-29 22:12:57 +02:00 (Migrated from github.com)

Modals should no longer be rendered directly inside of other components.

Acceptance Criteria

Definition of Done

  • Tested against acceptance criteria
  • Tested against the assumptions of the user story
  • The project builds without errors
  • Unit tests are written and passing
  • Tests on devices/browsers listed in the issue have passed
  • QA performed & issues resolved
  • Refactoring completed
  • Any configuration or build changes documented
  • Documentation updated
  • Peer Code Review performed
Modals should no longer be rendered directly inside of other components. ### Acceptance Criteria 1. 2. 3. ### Definition of Done - [ ] Tested against acceptance criteria - [ ] Tested against the assumptions of the user story - [ ] The project builds without errors - [ ] Unit tests are written and passing - [ ] Tests on devices/browsers listed in the issue have passed - [ ] QA performed & issues resolved - [ ] Refactoring completed - [ ] Any configuration or build changes documented - [ ] Documentation updated - [ ] Peer Code Review performed
liamcardenas commented 2017-11-20 21:32:48 +01:00 (Migrated from github.com)

@kauffj most appear to be moved over, do you know off the top of your head which ones are left? If not, I can look and find out.

@kauffj most appear to be moved over, do you know off the top of your head which ones are left? If not, I can look and find out.
kauffj commented 2017-11-20 22:43:39 +01:00 (Migrated from github.com)

Search for the following strings:

  • <Modal appearing anywhere outside of a modal view. Though potentially the ones in <SplashScreen> can or have to stay.
  • modal == " Anywhere a modal is being checked against a string it should be replaced with the appropriate constant from modal_types.js.
  • modal: " Same as above, but for dispatches.

There may be another signature to look for, but I think that's probably all of them.

(This is a great example of the type of problem that makes me very pro strong types, immutability, and whatever else makes your code easier to reason about.)

Search for the following strings: - `<Modal` appearing anywhere outside of a modal view. Though potentially the ones in `<SplashScreen>` can or have to stay. - `modal == "` Anywhere a modal is being checked against a string it should be replaced with the appropriate constant from `modal_types.js`. - `modal: "` Same as above, but for dispatches. There may be another signature to look for, but I think that's probably all of them. (This is a great example of the type of problem that makes me very pro strong types, immutability, and whatever else makes your code easier to reason about.)
alyssaoc commented 2018-08-28 19:41:15 +02:00 (Migrated from github.com)

@seanyesmunt can you confirm this is done?

@seanyesmunt can you confirm this is done?
neb-b commented 2018-09-06 02:16:47 +02:00 (Migrated from github.com)

There is one modal left, inside of rewardLink/view.jsx

Currently if claiming a reward through the reward link fails, we pass an error string into the component. We can just call doNotify inside of the redux action.

There is one modal left, inside of `rewardLink/view.jsx` Currently if claiming a reward through the reward link fails, we pass an error string into the component. We can just call `doNotify` inside of the redux action.
neb-b commented 2019-01-17 07:39:07 +01:00 (Migrated from github.com)

Finally removed the last one in https://github.com/lbryio/lbry-desktop/pull/2206

Finally removed the last one in https://github.com/lbryio/lbry-desktop/pull/2206
Sign in to join this conversation.
No milestone
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference: LBRYCommunity/lbry-desktop#637
No description provided.