microsoftgraph / microsoft-graph-toolkit

Authentication Providers and UI components for Microsoft Graph 🦒
https://docs.microsoft.com/graph/toolkit/overview
Other
921 stars 284 forks source link

Allow required attribute in people-picker mgt component #2988

Open SatishKumar-WSP opened 4 months ago

SatishKumar-WSP commented 4 months ago

Proposal: Allow required attribute in people-picker mgt component

Description

Allow required attribute in people-picker mgt component. It should validate on form submission, if it does not contains any value then form should mark as invalid like bootstrap do for normal HTML controls.

Rationale

Preferred Solution

Additional Context

sebastienlevert commented 4 months ago

Thanks for reporting this issue. It's something we've heard in the past and we should fix for both functionality but also accessibility reasons. Tagging @gavinbarron on work that was done in that space before.

gavinbarron commented 4 months ago

I'll disagree that this is an a11y concern but wholeheartedly agree that extending this custom element to expose a validty state seems like a great idea.

gavinbarron commented 4 months ago

I've done a little digging. We can tap into the native form validation by using a form associated control https://web.dev/articles/more-capable-form-controls https://css-tricks.com/creating-custom-form-controls-with-elementinternals/

The biggest drawback I've found thus far is that this strategy doesn't seem to enable the :required pseudo-selector. But otherwise this looks like a good way forward.

gavinbarron commented 4 months ago

@sebastienlevert can we have a spec around what validation elements we're going to support on the people picker? Should we enable a range for multi selection? Are there other validation states we should be handling? https://developer.mozilla.org/en-US/docs/Web/API/ValidityState What should our error messages be? When does the validation fire? Does the validation fire if there is no form? Does validation fire if the form sets the novalidate attribute?

sebastienlevert commented 4 months ago

My first comment would be to support it the same way the Fluent UI WC TextField works... https://fluent-components.azurewebsites.net/?path=/docs/components-text-field--text-field

Should we be the one to handle all those cases or proxy what's already available in Fluent?

SatishKumar-WSP commented 4 months ago

@sebastienlevert @gavinbarron may I know if there is any update on it please?

gavinbarron commented 4 months ago

My first comment would be to support it the same way the Fluent UI WC TextField works... https://fluent-components.azurewebsites.net/?path=/docs/components-text-field--text-field

Should we be the one to handle all those cases or proxy what's already available in Fluent?

I don't think that we can proxy what's in fluent as the use case here is somewhat different. The fluent case is checking the value of the text input inside the shadow dom. In this case we're checking the state of the selectedPeople property, which is distinct from the value in the input which we use as a search input, so having a value in the search input does not mean that the control would be in a valid state if it were required.

I think that there is value in looking at how FAST has implemented FACE components, but in this instance we'll be doing most of the work ourselves.

@SatishKumar-WSP we're working through the right design and specs for this feature request so that when we do start on building it we have a clear picture of what done looks like.

SatishKumar-WSP commented 4 months ago

@gavinbarron thanks for sharing the update. I am waiting for this feature to utilize it in the component.

SatishKumar-WSP commented 3 months ago

@gavinbarron Hope you are doing good! I am assuming it is still in progress and waiting for this feature. I appreciate if can you share update on it please?

Mnickii commented 3 months ago

@SatishKumar-WSP we are still doing the design work around this feature request and have not yet come up with a timeline to building it. Will share an update once we start building it.