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.

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

fix: unable to set initial boolean attributes on picker #6311

Closed
radium-v opened this issue Aug 23, 2022 · 3 comments · Fixed by #6773
Closed

fix: unable to set initial boolean attributes on picker #6311

radium-v opened this issue Aug 23, 2022 · 3 comments · Fixed by #6773
Assignees
Labels
area:fast-foundation Pertains to fast-foundation bug A bug closed:obsolete No longer valid status:planned Work is planned

Comments

@radium-v
Copy link
Collaborator

🐛 Bug Report

The filter-selected and filter-query boolean attributes on the <fast-picker> are true by default and the picker cannot be initialized in a state where they are not present.

💻 Repro or Code Sample

In this example, the picker is created and by default has the filter-selected and filter-query attributes present:

const picker = document.createElement("fast-picker");
document.body.append(picker);

Inserting a <fast-picker> element in HTML has the same effect:

<!-- Inserted markup -->
<fast-picker></fast-picker>

<!-- after DOM load -->
<fast-picker current-value="" selection="" filter-selected="" filter-query="" no-suggestions-text="No suggestions available" suggestions-available-text="Suggestions available" loading-text="Loading suggestions" menu-placement="bottom-fill"><fast-picker-list slot="list-region" role="list" class="picker-list"><!---->
    <input slot="input-region" role="combobox" type="text" autocapitalize="off" autocomplete="off" haspopup="list">
</fast-picker-list><fast-picker-menu role="list" slot="menu-region" id="listbox-0"><!----></fast-picker-menu></fast-picker>

In this example, the picker is created and does not have the attributes present when appended to the document body:

const picker = document.createElement("fast-picker");
picker.filterSelected = false;
picker.filterQuery = false;
document.body.append(picker);

🤔 Expected Behavior

The attributes filter-selected and filter-query should not be present when not explicitly set.

😯 Current Behavior

Since both filterSelected and filterQuery are both true at construction time, their boolean attributes are mapped to be present. There's no way to declare a <fast-picker> element without these attributes - property mutation after assignment and before connection is required.

💁 Possible Solution

Here are some possible solutions:

  • Use the booleanConverter converter to allow string values of true and false. This would break the defined API.
  • Set filterSelected and filterQuery to false by default. This would break existing implementations which don't already set these attributes in an explicit manner.

🔦 Context

This bug was found while converting the Karma tests to Playwright.

@radium-v radium-v added the status:triage New Issue - needs triage label Aug 23, 2022
@EisenbergEffect EisenbergEffect added bug A bug status:planned Work is planned area:fast-foundation Pertains to fast-foundation and removed status:triage New Issue - needs triage labels Aug 24, 2022
@EisenbergEffect EisenbergEffect added this to the FAST Foundation 3.0 milestone Aug 24, 2022
@EisenbergEffect
Copy link
Contributor

@radium-v I believe we made a general decision that all boolean attributes should be setup to be false by default. @chrisdholt can confirm/deny.

@chrisdholt
Copy link
Member

@scomea can you take a look at reworking picker to support defaulting to false.

@scomea scomea self-assigned this Jul 5, 2023
@janechu janechu removed this from the FAST Foundation 3.0 milestone May 28, 2024
@janechu
Copy link
Collaborator

janechu commented May 29, 2024

Unfortunately @microsoft/fast-foundation is being deprecated, refer to #6955. I see this was mentioned in a PR, we will be addressing open PRs and merging what we can before we snap an archive branch to preserve the latest state of Foundation, however to bring us up to date I am closing out issues.

@janechu janechu closed this as completed May 29, 2024
@janechu janechu added the closed:obsolete No longer valid label May 29, 2024
janechu pushed a commit that referenced this issue Jun 14, 2024
# Pull Request

## 📖 Description

As described in #6311 picker's "filter-selected" and "filter-query" attributes were incorrectly configured and always ended up being true unless set via the prop.  

This change resolves this issue by:
- remove "filter-query" and "filter-selected" attributes (never worked)
- introduces new `disableQueryFilter` and `disableSelectionFilter` props and associated attributes, defaults to false to match current default behavior
- deprecates the `filterQuery` and `filterSelected` props but keeps them in sync with the new props so we shouldn't break anyone who is setting these

This change also fixes an issue where the "no options available" display wasn't always showing up:

![image](https://github.com/microsoft/fast/assets/7649425/0702e75d-893a-494f-8723-d0ed1095582f)

When initialized with no options picker could open the menu without displaying the "no options available" message.  This pr simplifies the mechanism that shows that to ensure it shows up when needed.

### 🎫 Issues
closes #6311

## ✅ Checklist

### General
- [x] I have included a change request file using `$ yarn change`
- [ ] I have added tests for my changes.
- [x] I have tested my changes.
- [ ] I have updated the project documentation to reflect my changes.
- [x] I have read the [CONTRIBUTING](https://github.com/microsoft/fast/blob/master/CONTRIBUTING.md) documentation and followed the [standards](/docs/community/code-of-conduct/#our-standards) for this project.

### Component-specific
- [ ] I have added a new component
- [x] I have modified an existing component
- [ ] I have updated the [definition file](https://github.com/microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#definition)
- [ ] I have updated the [configuration file](https://github.com/microsoft/fast/blob/master/packages/web-components/fast-components/CONTRIBUTING.md#configuration)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:fast-foundation Pertains to fast-foundation bug A bug closed:obsolete No longer valid status:planned Work is planned
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants