Skip to content

feat: component graph node name toogle#797

Merged
antfu merged 9 commits intonuxt:mainfrom
runyasak:feat/component-name-toggle
Mar 5, 2025
Merged

feat: component graph node name toogle#797
antfu merged 9 commits intonuxt:mainfrom
runyasak:feat/component-name-toggle

Conversation

@runyasak
Copy link
Contributor

🔗 Linked issue

resolves #694

❓ Type of change

  • 📖 Documentation (updates to the documentation or readme)
  • 🐞 Bug fix (a non-breaking change that fixes an issue)
  • 👌 Enhancement (improving an existing functionality)
  • ✨ New feature (a non-breaking change that adds functionality)
  • 🧹 Chore (updates to the build process or auxiliary tools and libraries)
  • ⚠️ Breaking change (fix or feature that would cause existing functionality to change)

📚 Description

I added the PascalCase component name to the selectedLabel field in extra as the selected display label, while keeping label as the current node label.

Next, I implemented DataSet for nodes, allowing node labels to be updated by their ID.

I also updated the playground to reproduce this enhancement. If this update is unnecessary, I’m happy to remove it as needed.

Screenshot

Screen.Recording.2568-02-27.at.01.01.43.mov

If you have any feedback or suggestions, please don’t hesitate to let me know.


const isGrayedOut = searchDebounced.value && !rel.id.toLowerCase().includes(searchDebounced.value.toLowerCase())

const label = path.split('/').splice(-1)[0].replace(/\.\w+$/, '')
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't we use an utility function to infer a better name from the path, instead of relying on the selected state?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That's interesting! But I'm not sure what the best approach is to extract the component name from the path.

Could you review my function if I implement it like this?

function getComponentName(path: string) {
  const splitPath = path.split('/');
  const lastChunkPath = splitPath.splice(-1)[0].replace(/\.\w+$/, '');

  if (lastChunkPath === 'index') {
    return splitPath.splice(-2)[0].replace(/\.\w+$/, '')
  }

  return lastChunkPath
}

If my code is incorrect or if you have a better approach, please let me know.

@runyasak
Copy link
Contributor Author

runyasak commented Mar 2, 2025

@antfu I pushed my updated version using the getComponentName utility function, without relying on the selected state.

However, I’m not sure if this approach aligns with your preference.
Please let me know if you have any comments or suggestions, and I’ll update my code as soon as possible.

@antfu
Copy link
Member

antfu commented Mar 5, 2025

I am not sure if I understand why we need to update the label name on select and reset back. Shouldn't we just give the correct name at the first place?

I updated the PR to change that. Hope that makes sense to you (let me know if not). Thanks for the PR

@antfu antfu merged commit 2eb2a37 into nuxt:main Mar 5, 2025
1 of 2 checks passed
@runyasak
Copy link
Contributor Author

runyasak commented Mar 5, 2025

@antfu Thank you for reviewing my code.

I am not sure if I understand why we need to update the label name on select and reset back. Shouldn't we just give the correct name at the first place?

I was just following the approach to create the toggle feature as described in the issue #694.
However, I also initially thought about providing the correct name from the start, just like you suggested.

Would you like me to create a new PR to implement this approach?
I can revert the old code and update it to directly set the correct name as the node label.

@antfu
Copy link
Member

antfu commented Mar 5, 2025

I already change that in 3b5fd26 (#797)

@runyasak
Copy link
Contributor Author

runyasak commented Mar 5, 2025

Thank you so much.
I'm looking for the next contribution. 😁

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.

feat: component graph node name toogle

2 participants