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

fix(nav-bar): set ng-href on nav-item even if md-nav-href is empty #11488

Merged
merged 1 commit into from
Oct 24, 2018

Conversation

Splaktar
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?

  • A new npm i may pick up jasmine-core@2.99.1 which will cause many test failures since we don't currently support that version.
  • Setting md-nav-href="" on an md-nav-item doesn't apply the proper ng-href="" because of a bad if check.
  • The docs indicate that a navigation attribute must be defined on each md-nav-item, but the code doesn't throw an exception if this occurs. This can lead to unexpected behaviors like infinite loops or hanging async tasks.
  • The Closure properties are incorrect in a number of cases.

Issue Number:
Relates to #11485. Relates to #11486. Fixes #11487.

What is the new behavior?

  • set ng-href on nav-item even if md-nav-href is empty
  • fix closure properties to better represent the existing code
  • pin the version of jasmine-core since we don't support 2.99.1 yet
  • slightly improve the tone of the exceptions
  • pull in some karma configuration tweaks from Angular Material

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

fix closure properties to better represent the existing code
pin the version of jasmine-core since we don't support 2.99.1 yet
slightly improve the tone of the exceptions
pull in some karma configuration tweaks from Angular Material

Relates to #11485. Relates to #11486. Fixes #11487.
@Splaktar Splaktar added type: bug type: chore P1: urgent Urgent issues that should be addressed in the next minor or patch release. labels Oct 17, 2018
@Splaktar Splaktar added this to the 1.1.11 milestone Oct 17, 2018
@googlebot googlebot added the cla: yes PR author has signed Google's CLA: https://opensource.google.com/docs/cla/ label Oct 17, 2018
@Splaktar Splaktar added the pr: merge ready This PR is ready for a caretaker to review label Oct 17, 2018
@Splaktar Splaktar assigned mmalerba and unassigned josephperrott Oct 23, 2018
@mmalerba mmalerba merged commit e876eec into master Oct 24, 2018
@Splaktar Splaktar deleted the fixNavBarAndTests branch October 25, 2018 09:50
marosoft pushed a commit to marosoft/material that referenced this pull request Nov 11, 2018
…ngular#11488)

<!-- 
Filling out this template is required! Do not delete it when submitting a Pull Request! Without this information, your Pull Request may be auto-closed.
-->
## PR Checklist
Please check that your PR fulfills the following requirements:
- [x] The commit message follows [our guidelines](https://github.com/angular/material/blob/master/.github/CONTRIBUTING.md#-commit-message-format)
- [x] Tests for the changes have been added or this is not a bug fix / enhancement
- [x] Docs have been added, updated, or were not required

## PR Type
What kind of change does this PR introduce?
<!-- Please check the one that applies to this PR using "x". -->
```
[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?
- A new `npm i` may pick up `jasmine-core@2.99.1` which will cause many test failures since we don't currently support that version.
- Setting `md-nav-href=""` on an `md-nav-item` doesn't apply the proper `ng-href=""` because of a bad `if` check.
- The docs indicate that a navigation attribute **must** be defined on each `md-nav-item`, but the code doesn't throw an exception if this occurs. This can lead to unexpected behaviors like infinite loops or hanging async tasks.
- The Closure properties are incorrect in a number of cases.

<!-- Please describe the current behavior that you are modifying and link to one or more relevant issues. -->
Issue Number: 
Relates to angular#11485. Relates to angular#11486. Fixes angular#11487.

## What is the new behavior?

- set ng-href on nav-item even if md-nav-href is empty
- fix closure properties to better represent the existing code
- pin the version of jasmine-core since we don't support 2.99.1 yet
- slightly improve the tone of the exceptions
- pull in some karma configuration tweaks from Angular Material

## Does this PR introduce a breaking change?
```
[ ] Yes
[x] No
```
<!-- If this PR contains a breaking change, please describe the impact and migration path for existing applications below. -->
<!-- Note that breaking changes are highly unlikely to get merged to master unless the validation is clear and the use case is critical. -->

## Other information
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/ P1: urgent Urgent issues that should be addressed in the next minor or patch release. pr: merge ready This PR is ready for a caretaker to review type: bug type: chore
Projects
None yet
Development

Successfully merging this pull request may close these issues.

tests: broken in master due to some upstream change
4 participants