lion icon indicating copy to clipboard operation
lion copied to clipboard

fix: try awaiting render for missing name attributes

Open jorenbroekema opened this issue 3 years ago • 4 comments

What I did

Draft fix for issue described by @alangdm https://lit-and-friends.slack.com/archives/CJGFWJN9J/p1622445542027800

Apparently React or lit-labs/react first initialises the component with default values and assigns the properties on a second pass, so when the form element is first connected it still doesn't have a name

Before merging this we should add a test that specifically tests that a form element is able to register when its name attribute is set during render (update lifecycle in Lit perhaps), so that it can be expected after updateComplete but not before.

jorenbroekema avatar May 31 '21 11:05 jorenbroekema

⚠️ No Changeset found

Latest commit: f008e3bf7d050ccdeaef4e144828e177b78f5551

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

changeset-bot[bot] avatar May 31 '21 11:05 changeset-bot[bot]

Updated the other addFormElement override methods to be async and await super.addFormElement. One test still failing tho, couldn't figure it out in a timely manner

jorenbroekema avatar May 31 '21 11:05 jorenbroekema

I'm also experiencing this issue. Any update on what is needed to merge this?

lpoulter avatar Oct 06 '21 11:10 lpoulter

There was one failing test and I couldn't really figure out why it was failing, and I dont think anyone from the team reviewed this yet. I am not sure if I can find time to dive deeper into that test so if possible I would delegate it to maybe @tlouisse who is our form system maestro. Or if someone else figures it out, also works. I do think this should have some prio because as it is lion form doesn't work very well with lit react wrapper, although afaik it does with the other wrapper that I forgot the name of, "reactify" or something?

jorenbroekema avatar Oct 06 '21 13:10 jorenbroekema

This PR is abandoned, please reopen if you want to update it.

gerjanvangeest avatar Oct 04 '23 14:10 gerjanvangeest