LG-8349: Move AddressSearch component into own package#8386
LG-8349: Move AddressSearch component into own package#8386dawei-nava merged 12 commits intomainfrom
Conversation
...ipt/packages/document-capture/components/in-person-location-post-office-search-step.spec.tsx
Outdated
Show resolved
Hide resolved
dawei-nava
force-pushed
the
dwang/LG-8439-address-search-pkg
branch
from
72c3ab0 to
25c806a
Compare
allthesignals
left a comment
allthesignals
left a comment
There was a problem hiding this comment.
This isn't really "tested" automatically, but does it still build locally? You might have to run through the flow manually in order to see (although I suspect that would be caught the Rails feature test!)
There was a problem hiding this comment.
Right now, it only works with a full address. We may support zipcode only search in the future.
There was a problem hiding this comment.
This is true for getting real results, but right now all those services are mocked.
There was a problem hiding this comment.
Oh interesting... because this hooks into the broader frontend form validation system, we'll want to make sure that this can be an optional param... unsure whether this is required or not...
There was a problem hiding this comment.
Right now, this is built into the package... I don't think it can be configured from the outside. I wonder if making it configurable would be within the scope of this particular ticket...
There was a problem hiding this comment.
Good point, will remove it.
There was a problem hiding this comment.
Are we planning to publish this to NPM? This would be okay for how it's used in the IdP, but we wouldn't be able to publish this to NPM, since there are internal references to other packages which are not published (@18f/identity-components, @18f/identity-react-i18n, etc).
We'd probably need to create a prepackaged bundle of those internal dependencies for this to work in another project that wants to use the NPM package.
NavaTim
left a comment
NavaTim
left a comment
There was a problem hiding this comment.
I would like to see a plan described for how we intend to address @aduth's concern before moving forward with this PR.
Are we planning to publish this to NPM? This would be okay for how it's used in the IdP, but we wouldn't be able to publish this to NPM, since there are internal references to other packages which are not published (
@18f/identity-components,@18f/identity-react-i18n, etc).We'd probably need to create a prepackaged bundle of those internal dependencies for this to work in another project that wants to use the NPM package.
|
If it's helpful, @allthesignals and I did some pairing on this several months ago. Old, incomplete branch: https://github.com/18F/identity-idp/compare/aduth-try-publish-bundled-package |
|
@aduth @dawei-nava I think that LG-9164 adequately addresses the concern about coupling between this package and other IDP code. |
changelog: Internal, Code Structure, separate AddressSearch UI component as package.
Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com>
Co-authored-by: Tim Bradley <90272033+NavaTim@users.noreply.github.com>
dawei-nava
force-pushed
the
dwang/LG-8439-address-search-pkg
branch
from
b2aaf7f to
8bdb521
Compare
* Add support for per-component stylesheets changelog: Internal, Optimization, Reduce size of stylesheet assets for critical path * Forward required to use uswds-core conventionally * Override _uswds-core via internal load path * Remove unnecessary load path * Prioritize `--load-path` flag in Sass load path resolution * Add specs for component stylesheets * Add specs for StylesheetHelper * Rename clipboard component stylesheet to simple scss * Render stylesheet_once_tags everywhere application is rendered * Fix component preview for per-component stylesheet
* Updating how gpo pending is set in dummy profile * changelog [skip changelog]
5 participants
🎫 Ticket LG-8349
🛠 Summary of changes
Split AddressSearch into package
📜 Testing Plan
Following following steps.