Skip to content

Fix #178: Adding top-level links now works correctly#196

Merged
TejInaco merged 2 commits intoeclipse-editdor:masterfrom
Pranav-0440:fix/top-level-links-178
Feb 19, 2026
Merged

Fix #178: Adding top-level links now works correctly#196
TejInaco merged 2 commits intoeclipse-editdor:masterfrom
Pranav-0440:fix/top-level-links-178

Conversation

@Pranav-0440
Copy link
Copy Markdown
Contributor

This PR fixes issue #178 where adding a top-level link
did not work and the dialog remained open.

Fix:

  • Initialize td.links as an array when missing
  • Avoid mutating parsedTD directly
  • Properly update offlineTD via context

Tested:

  • Works when "links" does not exist
  • Works when "links": []
  • Dialog closes correctly

…orrectly

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>
Copilot AI review requested due to automatic review settings February 11, 2026 17:58
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR fixes issue #178 where adding a top-level link failed when the links property was missing or undefined in the Thing Description (TD). The dialog would remain open and console errors would occur. The fix properly initializes the links array, avoids direct mutation of state, and ensures the TD is correctly updated through the context API.

Changes:

  • Fixed top-level link addition by initializing td.links as an empty array when missing
  • Refactored to use structuredClone() to avoid direct mutation of context.parsedTD
  • Simplified component structure and improved code readability with modern React patterns

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +49 to +51
const [currentLinkedTd, setCurrentLinkedTd] = React.useState<
Record<string, any>
>({});
Copy link

Copilot AI Feb 11, 2026

Choose a reason for hiding this comment

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

Unused variable currentLinkedTd.

Suggested change
const [currentLinkedTd, setCurrentLinkedTd] = React.useState<
Record<string, any>
>({});

Copilot uses AI. Check for mistakes.
@egekorkan
Copy link
Copy Markdown
Contributor

@Pranav-0440 please make sure that you limit the changes seen in git diff to the problem area. there are other changes in your PR

@Pranav-0440
Copy link
Copy Markdown
Contributor Author

Thanks @egekorkan for the feedback. You’re right there are additional changes included. I’ll limit the diff strictly to the problem area and remove the unrelated modifications. I’ll update the PR shortly.

Signed-off-by: Pranav-0440 <pranavghorpade61@gmail.com>
@netlify
Copy link
Copy Markdown

netlify Bot commented Feb 13, 2026

Deploy Preview for editdor ready!

Name Link
🔨 Latest commit 3c8d6a4
🔍 Latest deploy log https://app.netlify.com/projects/editdor/deploys/698f6590fb39930008f6fea7
😎 Deploy Preview https://deploy-preview-196--editdor.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 project configuration.

@egekorkan egekorkan requested a review from TejInaco February 17, 2026 23:58
Copy link
Copy Markdown
Contributor

@TejInaco TejInaco left a comment

Choose a reason for hiding this comment

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

Good work.

@TejInaco TejInaco merged commit 3c6c4ad into eclipse-editdor:master Feb 19, 2026
9 of 10 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.

4 participants