Pfe-autocomplete does not respond to attributes updating in SPA context

[Djfaucette]

In a react app, the is-disabled attribute as well as other attributes are unusable due to a lack of mutation observer

For example, if you visit this url: https://catalog.redhat.com/software/applications/search?q=a the query string is passed, but the search form is disabled and the browser default "x" is shown instead of the component's "x" experience. Typing in the input remedies the issue, however these should be read from attributes and watched so that re-rendering is trivial. Attempting to add a disabled value in React led to an error that prevented rendering

Steps to reproduce

1. Go to 'catalog'
2. See the browser default "x" and disabled search button
3. type any additional input
4. See that autocomplete handles the changes

[bennypowers]

Thanks @Djfaucette can you point me in the direction of a source reproduction so we can better understand the issue?

Maybe starting from here? https://codesandbox.io/s/serene-dust-qvrkz?file=/src/App.js

[Djfaucette]

@bennypowers https://codesandbox.io/s/relaxed-https-03jlt?file=/src/App.js

Code sandbox sometimes ignores the error, but when I refresh in the editor I see the same error:

Cannot read properties of undefined (reading 'setAttribute')

[bennypowers]

nice. thanks for the repro. I will add or confirm that there are unit tests for this in 2.0 branch, and I will work on a patch script you can apply to affected 1.0 pages.

Tangentially - this problem is caused in the change observer for is-disabled attribute. in 2.0 branch, we changed that attr to disabled to be more in line with built-in- and custom-element conventions. The is-disabled attr will still work in 2.0 but it's deprecated, so prepare to update your patterns.

[bennypowers]

Having looked a little more deeply into your reproduction, I believe that this is ultimately a react issue. Read on for the exciting details! 💨

Note that react will set the is-disabled value to either the string value 'true' or the string value 'false'. As a result, PfeAutocomplete is disabled in all cases, since the string values 'true' and 'false' are both truthy.

So while there is some defensive programming work that could be done to improve <pfe-autocomplete> for cases like these, ultimately this is a react issue.

There are several potentially interesting solutions to react's idiosyncratic behaviour here:

1. Conditionally set the is-disabled attribute, for example by spreading props (codesandbox)

<pfe-autocomplete
  button-text="yadda yadda"
  { ...props.testData.disabled && { 'is-disabled': true } }>

2. Use the isDisabled DOM property

React < 18@experimental (codesandbox)

export default function App(props) {
  const autocompleteRef = createRef();

  useEffect(
    (x) => {
      autocompleteRef.current.isDisabled = props.testData.inputDisabled;
    },
    [autocompleteRef, props.testData.inputDisabled]
  );

  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      <h2>Start editing to see some magic happen!</h2>
      <pfe-autocomplete ref={autocompleteRef} button-text="Search">
        <input value={props.testData.searchQuery} />
      </pfe-autocomplete>
    </div>
  );
}

React >= 18@experimental (codesandbox)

export default function App(props) {
  return (
    <div className="App">
      <h1>Hello CodeSandbox</h1>
      <h2>Start editing to see some magic happen!</h2>
      <pfe-autocomplete
        isDisabled={props.testData.inputDisabled}
        button-text="Search"
      >
        <input value={props.testData.searchQuery} />
      </pfe-autocomplete>
    </div>
  );
}

3. Use the disabled attribute/property pair (PFE 2.0 branch only), since react hardcodes a special case for that attr

<pfe-autocomplete
  button-text="yadda yadda"
  disabled={props.testData.inputDisabled}>

[Djfaucette]

@bennypowers Nice! Thanks for the additional exploration.

I would assume that is-disabled="false" would be handled by the component since is-disabled=false still returns the value 'false' in HTML element.getAttribute() etc. If that were the case I think it would all be Gucci in a React app, change observer aside. WDYT?

Either way, many thanks for the detailed workarounds!

[bennypowers]

boolean attributes in HTML have very specific semantics. They are true when present and false when absent.

consider HTMLInputElement

https://codepen.io/bennyp/pen/mdBgwyM?editors=1011

<input disabled="true">
<input disabled="false">
<input disabled>
<input>

<script>
for (const input of document.querySelectorAll('input'))
  console.log(input.disabled, input.outerHTML);
</script>

true "<input disabled='true'>" true "<input disabled='false'>" true "<input disabled=''>" false "<input>"

We'd prefer not to make changes to custom elements for the sake of one highly specific, non-standard, semantics-breaking use case which would have unintuitive results for other users.

[Djfaucette]

Yeah, I guess the issue here is we can't pass a flag to the component to make the search work when there is no input.value even when the input is not required. That's the use case described above. Maybe we should just remove that default behavior?

After looking again, the input isn't disabled when there is no input.value but you still can't submit the form and there is no "off switch" for that behavior.

[bennypowers]

🤔 ok so there's more here under the covers than just "can the element load correctly with is-disabled"

Maybe we should set a time for a call, tmrw morning?