search

aurelia / dialog

A dialog plugin for Aurelia.
MIT License
106 stars 115 forks source link

fix attach-focus for dynamic change #346

Closed HIRANO-Satoshi closed 6 years ago

HIRANO-Satoshi commented 6 years ago

This fixes an issue that the focus did not move when JS changed focusedIndex in the following example.

<li repeat.for="index of keywords">
  <input attach-focus.bind="focusedIndex === index">
bigopon commented 6 years ago

Is it a bit unexpected to have attach focus even after init attached ? We already have quite standard behavior with focus attribute, i think we can use hat in place of dynamic attach focus, or call the logic in attach focus only once. Thought ? @StrahilKazlachev @633b92c

StrahilKazlachev commented 6 years ago

@HIRANO-Satoshi can you give some info if you've tried focus and if yes what it did not work with it?

StrahilKazlachev commented 6 years ago

@bigopon the more I look at this the more it looks like attach-focus is a lightweight focus.one-time, without the focus and blur listeners. Or am I missing something? Still will await on the use case feedback and push revert commit if needed.

bigopon commented 6 years ago

I just had a look a the code again, and the behavior was doing what it's supposed to do, focus once on after attached. I think for dynamic value scenarios that requires focus changes, it should be the job of a separate attribute.

bigopon commented 6 years ago

the example code in this PR can be changed to

<li repeat.for="index of keywords">
  <input focus.to-view="focusedIndex === index">
HIRANO-Satoshi commented 6 years ago

I have not noticed the standard way by focus.bind. Maybe we don't need this PR, if focus.bind works fine.

Sorry for bothering you.

bigopon commented 6 years ago

No worries, if you could confirm focus.to-view works fine for you, that'd be great. Please note that default binding mode of focus is two way, which means focus.bind may not be what you want for the purpose of focusing on attach only once.