Skip to content

Conversation

xiata
Copy link

@xiata xiata commented Dec 6, 2019

  • Fixes positioning code for the dropdown. ui-select-detached hosed things up with position: fixed.
  • removeGlobalHandlers and resetDropdown merged together since there is no reason not to do both.
  • Rearranged some functions and variables.
  • Aliasing of document.documentElement.
  • Dont bother resetting position of dropdown on close.
  • Fix setActiveIndexToSelected to handle track bys more complicated than track by key.id, supporting scoped properties such as track by option[col.colDef.field]

@xiata xiata added the Review Ready Review is ready label Dec 6, 2019
@xiata xiata requested a review from Joel-Kornbluh December 6, 2019 22:46
@xiata xiata changed the title Viv 730/fix calculation [VIV-730] Fix dropdown calculuation Dec 9, 2019
@xiata xiata changed the title [VIV-730] Fix dropdown calculuation [VIV-730] Fix dropdown calculation Dec 9, 2019
…n[col.colDef.field] usage (was expecting thing.id). (VIV-730)
@xiata xiata changed the title [VIV-730] Fix dropdown calculation [VIV-730] Fix dropdown calculation and complex track by Dec 9, 2019
@Joel-Kornbluh
Copy link

  1. In payroll grid when clicking ALT+DOWN to activate, the drop-down opens but isn't focused
  2. When drop-down isn't appended to body, it will sometimes remain aligned to the top (See attachment below) which causes some weird behavior when searching

image

@Joel-Kornbluh Joel-Kornbluh added Needs Work Needs work before merging is allowed and removed Review Ready Review is ready labels Dec 10, 2019
@xiata xiata added Review Ready Review is ready and removed Needs Work Needs work before merging is allowed labels Dec 10, 2019
@xiata
Copy link
Author

xiata commented Dec 10, 2019

Fixed, calculateDropdownPosition needed a timeout in refreshItems.

@Joel-Kornbluh
Copy link

@xiata Great job on this!
Tested the the new implementation in different edge cases. Works a charm with automatic positioning.

@Joel-Kornbluh Joel-Kornbluh merged commit 27de846 into master Dec 13, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Review Ready Review is ready
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants