fix(combobox): refresh behavior of the list and position #719
Conversation
| left: '0', | ||
| top: `${y}px`, | ||
| }); | ||
| if (!this.isOpen || !this.resultListElement) { |
There was a problem hiding this comment.
done twice (see first instruction of RaF)
| @@ -52,6 +52,7 @@ export class OdsCombobox implements OdsFormElement { | |||
| @State() private hasNoResults: boolean = false; | |||
| @State() private isInvalid: boolean | undefined; | |||
| @State() private isOpen: boolean = false; | |||
| @State() private items: ResultElement[] = []; | |||
There was a problem hiding this comment.
Looks like it is only meant to force a refresh of the element list when slot change.
Plus you just pass its ref to this.resultElements anyway.
If we do want to update the list without closing the list, we could just use the forceUpdate method to trigger a rerender.
I wonder even if it could be better to close and the reopen the dropdown when items change so that it isi visible by the user, otherwise the contents he is currently on, is suddenly modified (then the isOpen state will trigger the refresh)
| return; | ||
| } | ||
|
|
||
| this.resultListElement.classList.remove('ods-combobox__results--bottom', 'ods-combobox__results--top'); |
There was a problem hiding this comment.
should not be duplicated, idealy it should be done on close, and we should always close and reopen, but it's TBD as mentionned on the state comment.
| Object.assign(this.resultListElement.style, { | ||
| bottom: `${this.searchHeight}px`, | ||
| left: '0', | ||
| top: 'auto', |
| }); | ||
| } else { | ||
| Object.assign(this.resultListElement.style, { | ||
| bottom: 'auto', |
| @@ -366,6 +367,23 @@ export class OdsCombobox implements OdsFormElement { | |||
| this.closeDropdown(); | |||
| } | |||
|
|
|||
| private hasDiff(oldItems: ResultElement[], newItems: ResultElement[]): boolean { | |||
There was a problem hiding this comment.
would be nice to move it into the controller and add unit tests
| await inputEl.click(); | ||
| await page.waitForChanges(); | ||
|
|
||
| // Verify dropdown is open |
There was a problem hiding this comment.
keep only meaningful comments
| }); | ||
|
|
||
| it('should maintain correct positioning when reopening the dropdown', async() => { | ||
| // Similar setup as above |
There was a problem hiding this comment.
picture each tests as independant, if test above is removed one day, this comment gets wrong
| @@ -228,4 +306,4 @@ describe('ods-combobox rendering', () => { | |||
| expect(hasScroll).toBe(false); | |||
| }); | |||
| }); | |||
| }); | |||
| }); | |||
There was a problem hiding this comment.
check your IDE to avoid removing last line automatically (or we'll have a diff each time)
No description provided.