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: update ID on schema change #2307

Merged

Conversation

LukasBoll
Copy link
Contributor

This commit will update the IDs of all controls if the schema prop handed over to JSON Forms changes.

closes #2213

Copy link

netlify bot commented Mar 27, 2024

Deploy Preview for jsonforms-examples ready!

Name Link
🔨 Latest commit 65b252b
🔍 Latest deploy log https://app.netlify.com/sites/jsonforms-examples/deploys/660bb92027542e00086e7a44
😎 Deploy Preview https://deploy-preview-2307--jsonforms-examples.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

@coveralls
Copy link

coveralls commented Mar 27, 2024

Coverage Status

coverage: 84.816% (+0.01%) from 84.804%
when pulling 65b252b on LukasBoll:update_ID_on_schema_change
into bb7a255 on eclipsesource:master.

This commit will update the IDs of all controls if the schema prop handed over to JSON Forms changes.

closes eclipsesource#2213
Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @LukasBoll , thanks for the update :) Generally the changes now look good to me. Is there a reason that this PR is still marked as a draft?

@LukasBoll
Copy link
Contributor Author

Hi @lucas-koehler ,
I noticed a bug, where the application crashes when switching between some of the examples if you remove the key attribute in the Vue-example app.
I wanted to confirm, that these errors are not related to this commit before this PR is merged.
Since the same issue occurs on the master branch, I assume the issue is unrelated and will remove the draft status.

@LukasBoll LukasBoll marked this pull request as ready for review April 15, 2024 11:24
Copy link
Contributor

@lucas-koehler lucas-koehler left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@LukasBoll Alright makes sense. I also tested with the vue example app and switching between the person and scope examples successfully updated the id even when removing the key in the example app. Without this fix, this did not work.

@lucas-koehler lucas-koehler merged commit c0549f2 into eclipsesource:master Apr 16, 2024
8 checks passed
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.

Control.id doesn't get updated properly, if multiple forms are involved
3 participants