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

[Bug] Angular Inputs: Date Picker Input Calendar does not appear on click #1282

Merged
merged 1 commit into from Apr 10, 2019

Conversation

RVMendoza
Copy link
Collaborator

@RVMendoza RVMendoza commented Apr 9, 2019

What does this PR do?

When angular is built in --prod, angular date picker input does not work. This is because it's not being packaged correctly with the flag.

Date picker uses a third party library called tiny-date-picker. There are a two ways to bring in external libraries: Injection, and import modules.

In order to use injection, the library has to have typings, which tiny-date-picker does not have.

Instead of creating one, we are opting to use the second method of importing it for the sake of simplicity.

Concerns addressed:

  • There were concerns from @RCopeland that the import module method might an Angular anti-pattern.
  • Injection would be useful if we needed access to the class constructor of components and services
    • We don't need that for tiny-date-picker

At a high level, Angular reads the NgModule metadata during the bootstrap process and creates an injector capable of providing these dependencies through the class constructor of components and services throughout the application. -- angularfirst.com

Tiny Date picker doesn’t need any of these things.

  • Injection makes that library global
    • We don't need that for tiny-date-picker

Further reading:

Associated Issue

#1119

Please check off completed items as you work.

If a checklist item or section does not apply to your PR
then please remove it.

Code

  • Build Component in Spark Vanilla (Sass, HTML, JS)
  • Build Component in Spark Angular
  • Build Component in Spark React
  • Unit Testing in Spark Vanilla with npm run test (100% coverage, 100% passing)
  • Unit Testing in Spark Angular with gulp test-angular (100% coverage, 100% passing)
  • Unit Testing in Spark React with gulp test-react (100% coverage, 100% passing)

Browser Testing (current version and 1 prior)

  • Google Chrome
  • Google Chrome (Mobile)
  • Mozilla Firefox
  • Mozilla Firefox (Mobile)
  • Microsoft Internet Explorer 11 (only this specific version of IE)
  • Microsoft Edge
  • Apple Safari
  • Apple Safari (Mobile)

@RVMendoza RVMendoza added type: bug For defects in the system status: DO NOT MERGE Use on PRs that aren't ready to be merged js: angular For issues related to Angular packages labels Apr 9, 2019
@RVMendoza RVMendoza self-assigned this Apr 9, 2019
@RVMendoza
Copy link
Collaborator Author

fixes #1119

@netlify
Copy link

netlify bot commented Apr 9, 2019

@netlify
Copy link

netlify bot commented Apr 9, 2019

@RVMendoza RVMendoza added status: PR review Use this when a PR is ready for review and removed status: DO NOT MERGE Use on PRs that aren't ready to be merged labels Apr 10, 2019
@RVMendoza RVMendoza merged commit 8f48718 into staging Apr 10, 2019
@RVMendoza RVMendoza deleted the date-picker-ang-bug branch April 10, 2019 20:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
js: angular For issues related to Angular packages status: PR review Use this when a PR is ready for review type: bug For defects in the system
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants