Skip to content

Conversation

@PascalVorwerk
Copy link
Collaborator

Pull Request

📖 Description

I've added a javascript function that will add/remove the disabled option on the underlying shadow dom element that controls the clear button. Because the parent component can change this value, I was not sure if this check should be done each render or only on the first render, if so let me know!

For testing I found that the FluentSearch did not have any tests yet, so i have added a few. Sadly, the change i have made was hard to test because the property is only rendered in the shadowdom.. Some other attributes like 'AutoComplete' were also hard to test.

🎫 Issues

#3430

👩‍💻 Reviewer Notes

So a few questions for reviewers:

  • Should the disabled attribute be added/removed each render, or only on the first render? Right now the JSModule is imported each render.
  • Is there any way to include tests for the newly added change? If so, i would love to learn how 😄

📑 Test Plan

Run the project and fill in some value in on the FluentSearch page at the states section. Before you could delete any filled value by clicking where the 'button would be', but now you can't anymore.

✅ Checklist

General

  • I have added tests for my changes.
  • I have tested my changes.
  • I have updated the project documentation to reflect my changes.
  • I have read the CONTRIBUTING documentation and followed the standards for this project.

@dvoituron
Copy link
Collaborator

dvoituron commented Feb 23, 2025

Have you ever checked whether a CSS pointer-event: none; fixed this problem (on the "clear" button depending of a fluent-search attribute)? If so, maybe it's simpler and avoids a JS call.

@PascalVorwerk
Copy link
Collaborator Author

Have you ever checked whether a CSS pointer-event: none; fixed this problem (on the "clear" button depending of a fluent-search attribute)? If so, maybe it's simpler and avoids a JS call.

Oh no i haven't... let me try try out! Although, when reading the docs i am wondering if a point event is equal to a onclick event.

@PascalVorwerk
Copy link
Collaborator Author

Have you ever checked whether a CSS pointer-event: none; fixed this problem (on the "clear" button depending of a fluent-search attribute)? If so, maybe it's simpler and avoids a JS call.

Okay so point-events: none does work, but how would i go about adding this conditionally in the css if it should only be added when disabled or readonly is set to true? Cause if we put it always, you can't clear anymore if the input is not disabled or readonly

@dvoituron
Copy link
Collaborator

Have you ever checked whether a CSS pointer-event: none; fixed this problem (on the "clear" button depending of a fluent-search attribute)? If so, maybe it's simpler and avoids a JS call.

Okay so point-events: none does work, but how would i go about adding this conditionally in the css if it should only be added when disabled or readonly is set to true? Cause if we put it always, you can't clear anymore if the input is not disabled or readonly

Something like this...

fluent-search[readonly]::part(clear-button),
fluent-search[disabled]::part(clear-button) {
    pointer-events: none;
}

@PascalVorwerk
Copy link
Collaborator Author

Have you ever checked whether a CSS pointer-event: none; fixed this problem (on the "clear" button depending of a fluent-search attribute)? If so, maybe it's simpler and avoids a JS call.

Okay so point-events: none does work, but how would i go about adding this conditionally in the css if it should only be added when disabled or readonly is set to true? Cause if we put it always, you can't clear anymore if the input is not disabled or readonly

Something like this...

fluent-search[readonly]::part(clear-button),
fluent-search[disabled]::part(clear-button) {
    pointer-events: none;
}

Thanks! It worked and I have pushed the changes.

@vnbaaij vnbaaij enabled auto-merge (squash) February 24, 2025 12:38
@vnbaaij vnbaaij merged commit 87dc9c6 into microsoft:dev Feb 24, 2025
4 checks passed
@vnbaaij vnbaaij changed the title [FluentSearch] FluentSearch fix deletion if Disabled/Readonly [Search] Fix deletion if Disabled/Readonly Mar 5, 2025
@vnbaaij vnbaaij added this to the v4.11.6 milestone Mar 5, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants