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

fix(select): prevent dispose call on uninitialized properties #1340

Merged
merged 5 commits into from
Apr 8, 2019
Merged

fix(select): prevent dispose call on uninitialized properties #1340

merged 5 commits into from
Apr 8, 2019

Conversation

yggg
Copy link
Contributor

@yggg yggg commented Apr 2, 2019

Please read and mark the following check list before creating a pull request:

Short description of what this resolves:

Fixes case when component was destroyed before calling init hook.

@yggg yggg requested a review from Tibing April 2, 2019 08:49
@yggg yggg added this to In progress in Nebular via automation Apr 2, 2019
@yggg yggg moved this from In progress to Needs review in Nebular Apr 2, 2019
@yggg yggg changed the title fix(select): prevent call dispose on uninitialized properties fix(select): prevent dispose call on uninitialized properties Apr 3, 2019
@yggg yggg requested a review from nnixaa April 4, 2019 08:26
@@ -281,6 +281,11 @@ describe('Component: NbSelectComponent', () => {
expect(selectButton.textContent).toEqual(selectedOption.value.toString());
}));

it(`should not call dispose on initialized resources`, () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

on un initialized?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, fixed in d8a2d4e.

Nebular automation moved this from Needs review to Reviewer approved Apr 8, 2019
@codecov
Copy link

codecov bot commented Apr 8, 2019

Codecov Report

Merging #1340 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master    #1340      +/-   ##
==========================================
+ Coverage   81.13%   81.14%   +<.01%     
==========================================
  Files         232      232              
  Lines        7046     7048       +2     
  Branches      596      598       +2     
==========================================
+ Hits         5717     5719       +2     
  Misses       1152     1152              
  Partials      177      177
Impacted Files Coverage Δ
...mework/theme/components/select/select.component.ts 89.83% <100%> (+0.1%) ⬆️

@yggg yggg merged commit a7a158d into akveo:master Apr 8, 2019
Nebular automation moved this from Reviewer approved to Done Apr 8, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Nebular
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

2 participants