ni / nimble

The NI Nimble Design System
https://nimble.ni.dev
MIT License
29 stars 9 forks source link

Consider removing table focusedRecycleCallback #2202

Open msmithNI opened 1 week ago

msmithNI commented 1 week ago

🧹 Tech Debt

While wrapping up the table keyboard nav feature (and testing it with MenuButton column prototypes), we realized that the focusedRecycleCallback may now be redundant, since the keyboard nav code will focus the cell on table scroll (if the action menu button or focusable cell content was focused). That code that focuses the cell, will also result in the previously focused element being blurred. So, if there's nothing additional we need to do for any column types other than blur, we may be able to remove TableCellView focusedRecycleCallback() and Virtualizer notifyFocusedCellRecycling().

We may want to wait until the menu button / button column types are done, once we're sure that the current code will handle all the various cases (in conjunction with table scroll / other virtualizer updates):

PR discussion thread

m-akinc commented 1 week ago

Also update the table column HLD template.

msmithNI commented 6 days ago

Also see comment from #2210

Of note, we may be avoiding a potential error case related to this just due to the focusedRecycleCallback right now - if an anchor is focused when data changes, it'll be blurred, in which case focus resets to the cell. So we shouldn't end up in a case where cellContentIndex is out of bounds even if tabbableChildren changed (anchor link present in initial data, not in updated data).