Skip to content
This repository has been archived by the owner on Dec 8, 2022. It is now read-only.

Added event that fires when the search component's x is clicked #1013

Merged

Conversation

Blackbaud-TrevorBurch
Copy link
Member

No description provided.

@blackbaud-sky-savage
Copy link
Collaborator

Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: dddb79b
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/267699785

(Please note that this is a fully automated comment.)

@codecov-io
Copy link

codecov-io commented Aug 23, 2017

Codecov Report

Merging #1013 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff           @@
##           master   #1013   +/-   ##
======================================
  Coverage     100%    100%           
======================================
  Files         308     308           
  Lines        5592    5594    +2     
  Branches      704     704           
======================================
+ Hits         5592    5594    +2
Impacted Files Coverage Δ
src/modules/search/search.component.ts 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cb8e6d2...614800e. Read the comment docs.

@@ -73,6 +73,9 @@ export class SkySearchComponent implements OnDestroy, OnInit, OnChanges {
@Output()
public searchChange = new EventEmitter<string>();

@Output()
public searchClear = new EventEmitter();

Choose a reason for hiding this comment

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

The EventEmitter type defaults to any, but the consumer is not expecting to receive anything in the event handlers. Mind adding EventEmitter<void>(); to make it more clear?

Copy link
Member Author

Choose a reason for hiding this comment

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

Sure

@Blackbaud-SteveBrush Blackbaud-SteveBrush self-assigned this Aug 24, 2017
@blackbaud-sky-savage
Copy link
Collaborator

Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: dc5cfed
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/268078908

(Please note that this is a fully automated comment.)

@blackbaud-sky-savage
Copy link
Collaborator

Tests passed. Automated cross-browser testing via BrowserStack and Travis CI shows that the JavaScript changes in this pull request are: CONFIRMED

Commit: 614800e
Build details: https://travis-ci.org/blackbaud-sky-savage/skyux2/builds/268390670

(Please note that this is a fully automated comment.)

@Blackbaud-SteveBrush Blackbaud-SteveBrush merged commit 63d58ac into blackbaud:master Aug 25, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants