Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

(People Picker) Deterministic Suggestions Resolution #51

Merged
merged 2 commits into from
Nov 1, 2022
Merged

(People Picker) Deterministic Suggestions Resolution #51

merged 2 commits into from
Nov 1, 2022

Conversation

scottdurow
Copy link
Collaborator

With the current beta implementation of the People Picker, the comparison of suggestion results with the search text was giving unpredictable results due to the comparison of the first record using both the key and name. If there was an operation that took longer than the 8-second timeout, the results would not be displayed correctly.

Additionally, if a result set returned no matches, then the suggestions were never resolved.

This PR attempts to create a more deterministic approach to handling suggestion resolution for the following search approaches:

  1. Filtering directly in the Suggestions binding
  2. Filtering against a Dataverse data source in the Search event
  3. Filtering against a collection in the Search event
  4. Searching using a connector in the Search event

For the following scenarios:

  1. Search results return zero results
  2. Search results return an unchanged result (this means that no updateView will be triggered)
  3. Search results are re-triggered by constantly typing in the search box (search is de-bounced)

The debounce module is used to ensure that new requests, replace previous ones and any resolution of suggestions can be superseded by a new one.

The PR also includes some minor typo and consistency fixes.

Since PCF cannot be guaranteed to return only a single suggestion result for each event raised, there is an inherent debounce and timeout built into this implementation - this does result in a somewhat response times.

I would be interested in what you think.

@Ramakrishnan24689
Copy link
Collaborator

With the current beta implementation of the People Picker, the comparison of suggestion results with the search text was giving unpredictable results due to the comparison of the first record using both the key and name. If there was an operation that took longer than the 8-second timeout, the results would not be displayed correctly.

Additionally, if a result set returned no matches, then the suggestions were never resolved.

This PR attempts to create a more deterministic approach to handling suggestion resolution for the following search approaches:

  1. Filtering directly in the Suggestions binding
  2. Filtering against a Dataverse data source in the Search event
  3. Filtering against a collection in the Search event
  4. Searching using a connector in the Search event

For the following scenarios:

  1. Search results return zero results
  2. Search results return an unchanged result (this means that no updateView will be triggered)
  3. Search results are re-triggered by constantly typing in the search box (search is de-bounced)

The debounce module is used to ensure that new requests, replace previous ones and any resolution of suggestions can be superseded by a new one.

The PR also includes some minor typo and consistency fixes.

Since PCF cannot be guaranteed to return only a single suggestion result for each event raised, there is an inherent debounce and timeout built into this implementation - this does result in a somewhat response times.

I would be interested in what you think.

This seems far more reliable. Thanks Scott.

@Ramakrishnan24689 Ramakrishnan24689 merged commit 503fd0f into microsoft:beta Nov 1, 2022
Ramakrishnan24689 added a commit that referenced this pull request Nov 1, 2022
@Ramakrishnan24689 @scottdurow @denise-msft
October release prep (#50)
0802c55 
* Calendar Test Added - gitignore updated

* Removed Shimmer Coverage/ Updated Test Snaps

* Udpated Test for Progress Indicator

* fix:(Calendar) test failing due to dependency on today's date (#30)

* fix: (pivot) width in custom pages (#33)

chore: (pivot) npm update
- stay with pcf-scripts 1.14.2 due to webpack dependency issue

* fix: auto-height in custom pages (#32)

fix: onchange not fired when any outputs undefined
chore: npm update and test
chore: spelling mistake
chore: variable names should be camelCase

* Facepile component added (#35)

* Facepile component added

* reactdom-definition

* fixing typos

* Update Facepile.1033.resx

Adding preview tag

* Facepile propertyname changed (#48)

* Features/people picker (#46)

* PeoplePicker Added

* Code refactor and Documentation

* Updated readme, fixed typos.

* refactor: code optimization-onsearch/retries

* Tweaks

* refactor : render - pre-selected members

* Readme modified

* refactor : duplicate code removed

* refactor : personakey & presence type change

* refactor: presence return type fix

Co-authored-by: Scott Durow <scott_durow@hotmail.com>

* Searchbox (#49)

* Added SearchBox

* updated searchbox

* added comments to index

* renamed component.types.ts and updated searchbox

* added tests

* enhancement

Co-authored-by: Denise Moran <43950360+denisem-msft@users.noreply.github.com>

* test: test modified

* (People Picker) Deterministic Suggestions Resolution (#51)

* fixes issue with suggestions not resolving correctly
fixes other minor typos and consistency issues

* fix casing on peoplepicker file

Co-authored-by: Scott Durow <scott_durow@hotmail.com>
Co-authored-by: Denise Moran <43950360+denisem-msft@users.noreply.github.com>
Ramakrishnan24689 added a commit that referenced this pull request Jun 12, 2023
@Ramakrishnan24689 @scottdurow
@denise-msft @slaouist
Beta (#264)
a7ad805 
* Calendar Test Added - gitignore updated

* Removed Shimmer Coverage/ Updated Test Snaps

* Udpated Test for Progress Indicator

* fix:(Calendar) test failing due to dependency on today's date (#30)

* fix: (pivot) width in custom pages (#33)

chore: (pivot) npm update
- stay with pcf-scripts 1.14.2 due to webpack dependency issue

* fix: auto-height in custom pages (#32)

fix: onchange not fired when any outputs undefined
chore: npm update and test
chore: spelling mistake
chore: variable names should be camelCase

* Facepile component added (#35)

* Facepile component added

* reactdom-definition

* fixing typos

* Update Facepile.1033.resx

Adding preview tag

* Facepile propertyname changed (#48)

* Features/people picker (#46)

* PeoplePicker Added

* Code refactor and Documentation

* Updated readme, fixed typos.

* refactor: code optimization-onsearch/retries

* Tweaks

* refactor : render - pre-selected members

* Readme modified

* refactor : duplicate code removed

* refactor : personakey & presence type change

* refactor: presence return type fix

Co-authored-by: Scott Durow <scott_durow@hotmail.com>

* Searchbox (#49)

* Added SearchBox

* updated searchbox

* added comments to index

* renamed component.types.ts and updated searchbox

* added tests

* enhancement

Co-authored-by: Denise Moran <43950360+denisem-msft@users.noreply.github.com>

* test: test modified

* (People Picker) Deterministic Suggestions Resolution (#51)

* fixes issue with suggestions not resolving correctly
fixes other minor typos and consistency issues

* fix casing on peoplepicker file

* Searchbox - fix : custom page width (#52)

* Added SearchBox

* updated searchbox

* added comments to index

* renamed component.types.ts and updated searchbox

* added tests

* enhancement

* removed comments from searchbox tests

* fix : Addresses CustomPage Width

Co-authored-by: Denise Moran <43950360+denisem-msft@users.noreply.github.com>

* October release doc updates

* refactor: modified create-release-added npm ci

* Update Facepile.resx remove additional tags

* Searchbox (#56)

* updated readme

* fix: assigning class to searchbox

* feature : set focus

Co-authored-by: Denise Moran <43950360+denisem-msft@users.noreply.github.com>

* Creatorkit mehdis persona (#47)

* Person Component

* Adding missed resource properties

* Various updates fixing comments from code review

* Adding InitialsColors property

* removing unused import and fixing readme

* fix: component import fix

Co-authored-by: Ramakrishnan Raman <raram@microsoft.com>

* Feature/searchbox#82 (#131)

* feat:#82 - completed

* Feature/tooltip (#132)

* Added tooltipcontext property

* Updated strings

* feat:icon tooltip

Co-authored-by: Denise Moran <43950360+denisem-msft@users.noreply.github.com>

* feat:SubwayNav component added (#133)

* feat:SubwayNav component added

* feat:spinbutton (#134)

* included Spinbtn directory

* removing code

* duplicate directory mention

* adding subwaynav versioning stamp

* fix:custom page width issue (#145)

* feat:subwaynav showanimation property (#225)

* feat:subwaynav showanimation property

* updated tests

* fixes microsoft/powercat-creator-kit#285 (#226)

* fixes microsoft/powercat-creator-kit#316 (#228)

* feat: searchbox-defaultvalue-set-option (#230)

* feat: searchbox-defaultvalue-set-option

* fixes calendar language support (#231)

* fixes searchtext bound property (#234)

* fixes searchtext bound property

* updated testsnap

* feat: searchbox bordercolor property (#235)

---------

Co-authored-by: Scott Durow <scott_durow@hotmail.com>
Co-authored-by: Denise Moran <43950360+denisem-msft@users.noreply.github.com>
Co-authored-by: Mehdi Slaoui Andaloussi <slaouist@hotmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@Ramakrishnan24689 Ramakrishnan24689 Ramakrishnan24689 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
2 participants
@scottdurow @Ramakrishnan24689