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

feat(nzselect): 为nz-select添加tabindex属性 #2574

Closed
wants to merge 4 commits into from

Conversation

DrZhong
Copy link

@DrZhong DrZhong commented Dec 3, 2018

PR Checklist

Please check if your PR fulfills the following requirements:

PR Type

What kind of change does this PR introduce?

[ ] Bugfix
[x] Feature
[ ] Code style update (formatting, local variables)
[ ] Refactoring (no functional changes, no api changes)
[ ] Build related changes
[ ] CI related changes
[ ] Documentation content changes
[ ] Application (the showcase website) / infrastructure changes
[ ] Other... Please describe:

What is the current behavior?

Issue Number: N/A

What is the new behavior?

nz-select 添加tabindex属性绑定,用户在填写表单的时候可以指定nz-select的tabindex,这样用户在使用tab键切换表单的时候可以过滤一些不想要焦点的select

Does this PR introduce a breaking change?

[ ] Yes
[x] No

Other information

@netlify
Copy link

netlify bot commented Dec 3, 2018

Deploy preview for ng-zorro-master ready!

Built with commit 11581f2

https://deploy-preview-2574--ng-zorro-master.netlify.com

@codecov
Copy link

codecov bot commented Dec 3, 2018

Codecov Report

Merging #2574 into master will decrease coverage by 1.83%.
The diff coverage is 100%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master   #2574      +/-   ##
=========================================
- Coverage   97.34%   95.5%   -1.84%     
=========================================
  Files         552     495      -57     
  Lines       11587   12100     +513     
  Branches      827    1689     +862     
=========================================
+ Hits        11279   11556     +277     
+ Misses        195     172      -23     
- Partials      113     372     +259
Impacted Files Coverage Δ
...mponents/select/nz-select-top-control.component.ts 97.75% <100%> (-2.25%) ⬇️
components/select/nz-select.component.ts 96.77% <100%> (-1.24%) ⬇️
components/core/util/scroll-into-view-if-needed.ts 42.85% <0%> (-23.81%) ⬇️
components/core/util/logger/logger.service.ts 52.17% <0%> (-18.42%) ⬇️
components/core/wave/nz-wave.directive.ts 83.33% <0%> (-16.67%) ⬇️
components/date-picker/week-picker.component.ts 77.77% <0%> (-14.53%) ⬇️
components/cascader/nz-cascader-li.component.ts 81.25% <0%> (-13.2%) ⬇️
components/core/addon/string_template_outlet.ts 88% <0%> (-12%) ⬇️
...ponents/core/services/update-host-class.service.ts 88.88% <0%> (-11.12%) ⬇️
components/core/util/getMentions.ts 50% <0%> (-10%) ⬇️
... and 424 more

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 3e0e319...11581f2. Read the comment docs.

@@ -154,6 +154,7 @@ export class NzSelectComponent implements ControlValueAccessor, OnInit, AfterVie
/** https://github.com/angular/angular/pull/13349/files **/
// tslint:disable-next-line:no-any
@Input() compareWith = (o1: any, o2: any) => o1 === o2;
@Input() tabindex = 1;
Copy link
Contributor

Choose a reason for hiding this comment

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

The default value of tabindex attribute in HTML Living Standard is 0 and I think we should align with that.

Also note that it is not recommended to use positive tabindex in most cases.

Copy link
Author

Choose a reason for hiding this comment

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

yes,the default value shoud be 0,but sometimes it is necessary to skip it.

Copy link
Member

@vthinkxie vthinkxie left a comment

Choose a reason for hiding this comment

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

@vthinkxie
Copy link
Member

This PR will be closed due to no response for a long time, thanks for your contribution anyway.

@vthinkxie vthinkxie closed this Jul 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants