Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#9646 closed feature (fixed)

Core: Deprecate .focus( delay)

Reported by: jzaefferer Owned by:
Priority: blocker Milestone: 1.11.0
Component: ui.core Version: 1.10.3
Keywords: Cc:
Blocked by: Blocking:

Description

As discussed in Amsterdam, we only use the delayed focus call in dialog. We don't need to modify core methods for just two calls. We should just use _delay() to delay the focus call.

Change History (10)

comment:1 Changed 3 years ago by jzaefferer

  • Status changed from new to open

comment:2 Changed 3 years ago by scottgonzalez

  • Milestone changed from none to 1.11.0
  • Summary changed from Dialog: Inline delay(x) from core to dialog to Core: Deprecate .focus( delay)
  • Type changed from bug to feature

comment:3 Changed 3 years ago by scottgonzalez

  • Component changed from ui.dialog to ui.core

comment:4 Changed 3 years ago by scottgonzalez

  • Priority changed from minor to blocker

comment:5 Changed 3 years ago by jzaefferer

We have a ton of usage within our unit tests. Updating all those will be a PITA. Can we move the proxied focus method to tests/unit/testsuite.js and leave unit tests as-is? That way we still only have to update dialog.

comment:6 Changed 3 years ago by jzaefferer

My earlier comment was very wrong, I tested the wrong thing. There's two calls in dialog, that's all there is. I've got a patch locally that makes .focus(n) throw an error ("Delayed focus is deprecated, use timeouts and call focus yourself"), along with removing the tests for the proxied method and replacing the usage in dialog. That's probably not good enough for deprecation, and while I remember seeing something about not using $.ui.backCompat anymore, I currently don't know what the right approach for this particular case is.

For rerefence, gist-diff of the patch: https://gist.github.com/jzaefferer/0cc41a3c2fd12494d139

comment:7 Changed 3 years ago by scottgonzalez

Throwing an error is never valid for deprecation, the functionality needs to still work otherwise it's not deprecated. We're still using $.uiBackCompat for now.

comment:9 Changed 3 years ago by Jörn Zaefferer

  • Resolution set to fixed
  • Status changed from open to closed

Core: Deprecate .focus( n ), replace in dialog with explicit timeouts

Fixes #9646

Changeset: df6110c0d424ff3306fdd5576011f2dcf4d242d0

comment:10 Changed 3 years ago by Jörn Zaefferer

Dialog: Fix shift-tab handling, focus the correct element

Copy-paste error introduced in df6110c0d424ff3306fdd5576011f2dcf4d242d0

Updates the tabbing test to be more specific about which element should have focus, instead of only checking if focus is within the dialog.

Ref #9646 Ref #10103 Closes gh-1264

Changeset: a0b84767a76098cdcc6375dfe28a7fee866bd395

Note: See TracTickets for help on using tickets.