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(typeahead): fix focus after leaving control #4622

Merged
merged 3 commits into from
Jan 11, 2020
Merged

fix(typeahead): fix focus after leaving control #4622

merged 3 commits into from
Jan 11, 2020

Conversation

tarusin
Copy link
Contributor

@tarusin tarusin commented Sep 18, 2018

Related to: #1884
Added new demo with input which demonstrates behavior with closed dropdown when we leaving input field.

@ghost ghost added the needs review label Sep 18, 2018
@codecov
Copy link

codecov bot commented Sep 28, 2018

Codecov Report

Merging #4622 into development will increase coverage by 18.92%.
The diff coverage is 66.66%.

Impacted file tree graph

@@               Coverage Diff                @@
##           development    #4622       +/-   ##
================================================
+ Coverage        52.52%   71.44%   +18.92%     
================================================
  Files                3      259      +256     
  Lines               99     8384     +8285     
  Branches            17     1563     +1546     
================================================
+ Hits                52     5990     +5938     
- Misses              37     1986     +1949     
- Partials            10      408      +398
Impacted Files Coverage Δ
src/typeahead/typeahead.directive.ts 74.61% <66.66%> (ø)
demo/src/app/docs/api-docs/analytics/analytics.ts 38.09% <0%> (-4.33%) ⬇️
demo/src/app/app.component.ts 36.11% <0%> (-4.32%) ⬇️
demo/src/polyfills.ts 100% <0%> (ø) ⬆️
src/sortable/index.ts 100% <0%> (ø)
src/chronos/utils/date-getters.ts 59.09% <0%> (ø)
src/index.ts 100% <0%> (ø)
src/chronos/create/valid.ts 90% <0%> (ø)
src/modal/bs-modal-ref.service.ts 75% <0%> (ø)
src/buttons/button-radio-group.directive.ts 52.63% <0%> (ø)
... and 250 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 6adf5f0...9f0ac01. Read the comment docs.

@Domainv
Copy link
Contributor

Domainv commented Sep 28, 2018

code coverage

@milosdanilov
Copy link

@tarusin @Domainv Is this going to be merged soon?

@Domainv
Copy link
Contributor

Domainv commented Mar 1, 2019

@milosdanilov Not sure, the PR require improvements

@Domainv Domainv added this to the 5.1.0 milestone Jun 10, 2019
@ejasarevic
Copy link

Any progress/plan for this PR?

@daniloff200
Copy link
Contributor

@EmanJasarevic oh, that's pretty old one
I hope to have time to look at it, and rework soon. Let's hope that it will be available in next or next two releases

@daniloff200 daniloff200 changed the base branch from development to 4622-branch January 11, 2020 10:18
@daniloff200 daniloff200 merged commit 938b9a5 into valor-software:4622-branch Jan 11, 2020
daniloff200 added a commit that referenced this pull request Jan 16, 2020
Co-authored-by: Dmitriy Danilov <daniloff200@gmail.com>
daniloff200 added a commit that referenced this pull request Feb 5, 2020
* fix(typeahead): fix focus after leaving control (#4622)

Co-authored-by: Dmitriy Danilov <daniloff200@gmail.com>

* fix(typeahead): fix wrong imports, update descrtiption, move property to config

* fix(typeahead): rename property, rename all demos

Co-authored-by: Ilya Tarusin <tarusin.iliya@gmail.com>
Co-authored-by: dmitry-zhemchugov <44227371+dmitry-zhemchugov@users.noreply.github.com>
@ejasarevic
Copy link

Hi @daniloff200,
thanks for making these changes. However, IMO it still behaves differently than expected (even demo example https://valor-software.com/ngx-bootstrap/#/typeahead#cancel-on-focus-lost).
Here are a couple of use cases which I've tested, correct me if I have wrong expectations:

  1. Start typing something (e.g "Wash") and wait for the async data to be loaded
    Expected: Data loaded properly, all good.
    Result: ✔️
  2. Start typing "Wash" and pres tab a couple of times to move the focus from typeahead:
    Expected: Typeahead will cancel async request and not load suggestions ("Washington" in this case).
    Result: ➖
  3. Start typing "Wash" and click outside of the input:
    Expected: Typeahead will cancel async request and not load suggestions ("Washington" in this case).
    Result: ➖

In our app we use the typeahead to provide suggestions, but also allow user to enter freetext in the input as well and accept it even though it is not option from the typeahead. For example, You start typing "Wash." and press enter/tab, focus will move to the next control and your input will be accepted. Is it doable with the current Typeahead implementation?

Regards

@chanageller
Copy link
Contributor

@daniloff200
This still seems to be broken. cancelRequestOnFocusLost is always set to false.
There seems to be a bug in the code, when assigning the variables with the config settings the naming is wrong.

line 189 in typeahead directive-

  {
    typeaheadHideResultsOnBlur: config.hideResultsOnBlur,
    typeaheadCancelRequestOnFocusLost: config.cancelRequestOnFocusLost,
    typeaheadSelectFirstItem: config.selectFirstItem,
    typeaheadIsFirstItemActive: config.isFirstItemActive,
    typeaheadMinLength: config.minLength,
    adaptivePosition: config.adaptivePosition,
    isAnimated: config.isAnimated
  }
);`

typeaheadCancelRequestOnFocusLost should be cancelRequestOnFocusLost

@daniloff200
Copy link
Contributor

woah
@chanageller can you submit a PR with that, and then we can check that?

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.

7 participants