Skip to content
This repository was archived by the owner on Sep 5, 2024. It is now read-only.

fix(nav-bar): null check tabs when finding tab #11964

Merged
merged 5 commits into from
Jul 17, 2020
Merged

fix(nav-bar): null check tabs when finding tab #11964

merged 5 commits into from
Jul 17, 2020

Conversation

tomaszgrabowski
Copy link
Contributor

PR Checklist

Please check that your PR fulfills the following requirements:

  • The commit message follows our guidelines
  • Tests for the changes have been added or this is not a bug fix / enhancement
  • Docs have been added, updated, or were not required

PR Type

What kind of change does this PR introduce?

[x] Bugfix
[ ] Enhancement
[ ] Documentation content changes
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Currently on browser window resize there is a possibility that tabs will not be found on time. This will be ended wit console error Cannot read property 'length' of null

Issue Number:

What is the new behavior?

No error

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@googlebot
Copy link

Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

📝 Please visit https://cla.developers.google.com/ to sign.

Once you've signed (or fixed any issues), please reply here with @googlebot I signed it! and we'll verify it.


What to do if you already signed the CLA

Individual signers
Corporate signers

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added the cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ label Jul 16, 2020
Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

Thank you for this issue report and contribution!

Can you please update the JSDoc and sign the CLA?

@Splaktar Splaktar added this to the 1.2.0 milestone Jul 16, 2020
@Splaktar Splaktar self-assigned this Jul 16, 2020
@Splaktar Splaktar added P2: required Issues that must be fixed. type: bug in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs labels Jul 16, 2020
Splaktar added a commit that referenced this pull request Jul 16, 2020
- `MdNavBarController._getTabs()` returns an empty array instead of `null`
  to match existing return type
- rename `MdNavItemController._focused` to `MdNavItemController.isFocused`
  - make it non-private since it is accessed by `MdNavBarController.onFocus()`
- fix/improve types
- remove unused variables
- add JSDoc

Relates to #11964
Splaktar added a commit that referenced this pull request Jul 16, 2020
- `MdNavBarController._getTabs()` returns an empty array instead of `null`
  to match existing return type
- rename `MdNavItemController._focused` to `MdNavItemController.isFocused`
  - make it non-private since it is accessed by `MdNavBarController.onFocus()`
- fix/improve types
- remove unused variables
- add JSDoc

Relates to #11964
@tomaszgrabowski
Copy link
Contributor Author

@googlebot I signed it!

@googlebot
Copy link

We found a Contributor License Agreement for you (the sender of this pull request), but were unable to find agreements for all the commit author(s) or Co-authors. If you authored these, maybe you used a different email address in the git commits than was used to sign the CLA (login here to double check)? If these were authored by someone else, then they will need to sign a CLA as well, and confirm that they're okay with these being contributed to Google.
In order to pass this check, please resolve this problem and then comment @googlebot I fixed it.. If the bot doesn't comment, it means it doesn't think anything has changed.

ℹ️ Googlers: Go here for more info.

@googlebot
Copy link

CLAs look good, thanks!

ℹ️ Googlers: Go here for more info.

@googlebot googlebot added cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ and removed cla: no PR author needs to sign Google's CLA: https://opensource.google.com/docs/cla/ labels Jul 17, 2020
@tomaszgrabowski
Copy link
Contributor Author

Thank you for this issue report and contribution!

Can you please update the JSDoc and sign the CLA?

@Splaktar required changes uploaded. Thanks for the review.

Splaktar added a commit that referenced this pull request Jul 17, 2020
- `MdNavBarController._getTabs()` returns an empty array instead of `null`
  to match existing return type
- rename `MdNavItemController._focused` to `MdNavItemController.isFocused`
  - make it non-private since it is accessed by `MdNavBarController.onFocus()`
- fix/improve types
- remove unused variables
- add JSDoc

Relates to #11964
@Splaktar Splaktar removed the in progress Mainly for in progress PRs, but may be used for issues that require multiple PRs label Jul 17, 2020
Copy link
Member

@Splaktar Splaktar left a comment

Choose a reason for hiding this comment

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

LGTM

@Splaktar
Copy link
Member

CI is broken due to NPM being down atm.

@Splaktar Splaktar added the pr: merge ready This PR is ready for a caretaker to review label Jul 17, 2020
@Splaktar Splaktar requested a review from andrewseguin July 17, 2020 21:31
@Splaktar Splaktar added the pr: lgtm This PR has been approved by the reviewer label Jul 17, 2020
@Splaktar Splaktar merged commit 91300ec into angular:master Jul 17, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ P2: required Issues that must be fixed. pr: lgtm This PR has been approved by the reviewer pr: merge ready This PR is ready for a caretaker to review type: bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants