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(select): keyboard support #1417

Merged
merged 12 commits into from
Apr 25, 2019
Merged

feat(select): keyboard support #1417

merged 12 commits into from
Apr 25, 2019

Conversation

yggg
Copy link
Contributor

@yggg yggg commented Apr 24, 2019

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

Short description of what this resolves:

Select could be opened with up and down arrows.
Options became focusable. It's possible to select an option with enter
and space keys.
This way option text is styled as option text not as input label.
@yggg yggg requested a review from nnixaa April 24, 2019 16:26
@codecov
Copy link

codecov bot commented Apr 25, 2019

Codecov Report

Merging #1417 into next will decrease coverage by 0.05%.
The diff coverage is 77.27%.

@@            Coverage Diff             @@
##             next    #1417      +/-   ##
==========================================
- Coverage   82.62%   82.57%   -0.06%     
==========================================
  Files         238      240       +2     
  Lines        7293     7328      +35     
  Branches      651      654       +3     
==========================================
+ Hits         6026     6051      +25     
- Misses       1079     1088       +9     
- Partials      188      189       +1
Impacted Files Coverage Δ
src/framework/theme/components/cdk/index.ts 100% <100%> (ø) ⬆️
...ramework/theme/components/cdk/keycodes/keycodes.ts 100% <100%> (ø)
...ork/theme/components/cdk/a11y/focus-key-manager.ts 100% <100%> (ø)
...mework/theme/components/select/select.component.ts 87.25% <70%> (-2.58%) ⬇️
...mework/theme/components/select/option.component.ts 98.21% <88.88%> (-1.79%) ⬇️

@@ -672,7 +670,19 @@ export class NbSelectComponent<T> implements OnInit, AfterViewInit, AfterContent

show() {
if (this.isHidden) {
if (!this.ref) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's not put if in if. Please, move in the separate function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed c8822b2

this.ref.attach(this.portal);
if (this.selectionModel.length) {
Copy link
Member

Choose a reason for hiding this comment

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

Let's not put if in if. Please, move in the separate function.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed c8822b2

@yggg yggg requested a review from Tibing April 25, 2019 15:01
@yggg yggg merged commit b6f6d78 into akveo:next Apr 25, 2019
yggg added a commit that referenced this pull request May 27, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants