-
Notifications
You must be signed in to change notification settings - Fork 23
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
fix(stark-ui): Ui: replace text-mask to imask.js #3384
Conversation
packages/stark-ui/src/modules/date-picker/components/date-picker.component.spec.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Dear @mhenkens
Thanks for your first PR on Stark. It seems great ! 👍
I made some comments for some parts of the code to adapt.
Please pay attention to document any property / class / method defined in your code. It is important for the documentation available on https://stark.nbb.be/api-docs/stark-ui/latest/index.html. FYI you can check the result of the documentation by running the command npm run docs:stark-ui:serve
from the root folder of Stark.
In order to simplify the final CHANGELOG of the next version of Stark, could you rebase and squash your commits as documented in the CONTRIBUTING guide: https://github.com/NationalBankBelgium/stark/blob/master/CONTRIBUTING.md#proposing-your-changes-by-submitting-a-pull-request-pr ?
At the end, you should have only 2 commits, one for the new feature (replacement of angular2-text-mask
by imask
in stark-ui
), the other one for the changes in the showcase in order to use the new implementation of the directive.
Also, it would be interesting that you work on another branch than your master branch for this feature in order to be able to work on another feature or bugfix in parallel 😊
You can check the documentation about our workflow on https://github.com/NationalBankBelgium/stark/blob/master/CONTRIBUTING.md#workflow
Alexis
packages/stark-ui/src/modules/date-picker/components/date-picker.component.spec.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/date-picker/components/date-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/date-picker/components/date-picker.component.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/date-picker/date-picker.module.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/input-mask-directives/directives/text-mask.constant.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/input-mask-directives/directives/text-mask.directive.spec.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/input-mask-directives/directives/text-mask.directive.spec.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/input-mask-directives/directives/text-mask.directive.spec.ts
Outdated
Show resolved
Hide resolved
packages/stark-ui/src/modules/input-mask-directives/directives/text-mask.directive.spec.ts
Outdated
Show resolved
Hide resolved
When changing language we got an issue with the name of the column for the Actions in NL Test are updated as well. ISSUES CLOSED: NationalBankBelgium#3391
This reverts commit 8dab8e4.
Kudos, SonarCloud Quality Gate passed! |
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
Issue Number: #2564
What is the new behavior?
Does this PR introduce a breaking change?
Other information