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

new Events and localization for AttachResult and ResultsFilter components #28

Merged
merged 4 commits into from
Aug 20, 2019

Conversation

mikegron
Copy link
Collaborator

Advise if I should add all events/strings in a single file for every component instead ?

Added strings for defaults strings of AttachResult and ResultsFilter
Added events when attaching / detaching result and filtering
SNOW-182

Added events when attaching / detaching result and filtering
SNOW-182
@louis-bompart
Copy link
Collaborator

Take the habit of creating branches from your fork :P

Copy link
Collaborator

@louis-bompart louis-bompart left a comment

Choose a reason for hiding this comment

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

Some changes/interrogations.
Also, please add JSDOC on all exported and public stuff

tests/components/ResultsFilter/ResultsFilter.spec.ts Outdated Show resolved Hide resolved
tests/components/ResultsFilter/ResultsFilter.spec.ts Outdated Show resolved Hide resolved
src/components/ResultsFilter/Strings.ts Show resolved Hide resolved
src/components/AttachResult/Strings.ts Show resolved Hide resolved
Added side effects files
SNOW-182
@louis-bompart louis-bompart self-requested a review August 20, 2019 15:42
Copy link
Collaborator

@louis-bompart louis-bompart left a comment

Choose a reason for hiding this comment

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

Some documentation missing and some questions about some tests

src/components/AttachResult/AttachResult.ts Show resolved Hide resolved
src/components/ResultsFilter/ResultsFilter.ts Show resolved Hide resolved
tests/components/ResultsFilter/ResultsFilter.spec.ts Outdated Show resolved Hide resolved
tests/components/ResultsFilter/ResultsFilter.spec.ts Outdated Show resolved Hide resolved
src/components/AttachResult/AttachResult.ts Show resolved Hide resolved
@mikegron mikegron merged commit 9abe803 into master Aug 20, 2019
@mikegron mikegron deleted the attach-events branch August 20, 2019 18:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants