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

feat: Add custom link modal #250

Merged
merged 23 commits into from
Oct 6, 2021
Merged

feat: Add custom link modal #250

merged 23 commits into from
Oct 6, 2021

Conversation

suzubara
Copy link
Contributor

@suzubara suzubara commented Oct 4, 2021

Description

This PR uses a branch of ReactUSWDS that adds the Modal component (trussworks/react-uswds#1622) and sets up a modal root element so we can render modals in the portal app. It builds off #249 to add a modal to that flow where a user can enter a label for a custom link.

Fixes #246

Review Notes

  • Verify you can add a link to an existing collection and be prompted to enter a label for the link.
  • The link label is a required field and you cannot save a link without entering something.
  • Since the link is only added in the local cache, refreshing the page will reset any changes you've made.
addLink.mp4

}

return (
<ModalPortal>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

All Modal components should be wrapped in this <ModalPortal> component, in order to be rendered into the modal-root element.

@github-actions
Copy link
Contributor

github-actions bot commented Oct 4, 2021

🎉 View the deployed prototype - https://246-add-modals--ussf-portal-client.netlify.app

@jbecker01
Copy link
Contributor

jbecker01 commented Oct 4, 2021

@suzubara There's a bug where:

  • I add a new custom link, give it a title and save it
  • I delete the new custom link
  • When I click on the Add Site button again, I'm immediately presented with the link name modal instead of being able to add the URL first.
  • If I add a name and then click save, a new link is created with the URL that I just deleted.

Would you like a new issue for this?

@suzubara
Copy link
Contributor Author

suzubara commented Oct 5, 2021

@suzubara There's a bug where:

  • I add a new custom link, give it a title and save it
  • I delete the new custom link
  • When I click on the Add Site button again, I'm immediately presented with the link name modal instead of being able to add the URL first.
  • If I add a name and then click save, a new link is created with the URL that I just deleted.

Would you like a new issue for this?

@jbecker01 nice find!! No thanks, I will resolve it in this PR

@abbyoung
Copy link
Contributor

abbyoung commented Oct 5, 2021

Another bug that seems related to John's:

  • Successfully add bookmark
  • Click Add Link to add another
  • Get the 'We don't recognize that link' modal immediately without entering a link
  • If I enter a label, it adds with the previous url

@suzubara
Copy link
Contributor Author

suzubara commented Oct 5, 2021

@jbecker01 @abbyoung both of the above issues should be resolved now!

Base automatically changed from 233-add-link-to-collection to main October 5, 2021 17:19
@@ -0,0 +1,14 @@
import { render, RenderOptions } from '@testing-library/react'

export const renderWithModalRoot = (
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Use this when writing Jest tests for modal components (or components that render modals)

@github-actions
Copy link
Contributor

github-actions bot commented Oct 5, 2021

🎉 View the deployed prototype - https://246-add-modals--ussf-portal-client.netlify.app

@abbyoung
Copy link
Contributor

abbyoung commented Oct 5, 2021

Jest: "global" coverage threshold for functions (95%) not met: 94.9% Thanks, I hate it! 😭

jbecker01
jbecker01 previously approved these changes Oct 5, 2021
Copy link
Contributor

@jbecker01 jbecker01 left a comment

Choose a reason for hiding this comment

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

Looks good to me!

@github-actions
Copy link
Contributor

github-actions bot commented Oct 6, 2021

🎉 View the deployed prototype - https://246-add-modals--ussf-portal-client.netlify.app

Copy link
Contributor

@abbyoung abbyoung left a comment

Choose a reason for hiding this comment

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

looks great! 🎉

@suzubara suzubara merged commit 8dfc7af into main Oct 6, 2021
@suzubara suzubara deleted the 246-add-modals branch October 6, 2021 22:21
@suzubara suzubara mentioned this pull request Dec 20, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Set up Modals
3 participants