Skip to content

Conversation

@chubakueno
Copy link
Contributor

@chubakueno chubakueno commented May 6, 2025

Purpose

Add icons to cluster and single autocomplete, refactor code to partially decouple from NodeSearchElementViewModel and NodeSearchElement.

2025-05-06.15-29-20.mp4

Declarations

Check these if you believe they are true

  • The codebase is in a better state after this PR
  • Is documented according to the standards
  • The level of testing this PR includes is appropriate
  • User facing strings, if any, are extracted into *.resx files
  • All tests pass using the self-service CI.
  • Snapshot of UI changes, if any.
  • Changes to the API follow Semantic Versioning and are documented in the API Changes document.
  • This PR modifies some build requirements and the readme is updated
  • This PR contains no files larger than 50 MB

Release Notes

Add icons to cluster and single autocomplete, partially decouple DNA code from preexisting single autocomplete code.

Reviewers

@johnpierson
@BogdanZavu
@QilongTang

FYIs

(FILL ME IN, Optional) Names of anyone else you wish to be notified of

@chubakueno chubakueno changed the title Add single and cluster icons, refactor DYN-8541: Add single and cluster icons, refactor May 6, 2025
Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

See the ticket for this pull request: https://jira.autodesk.com/browse/DYN-8541

@johnpierson
Copy link
Member

I keep seeing this after a fresh rebuild with your PR:
image

@johnpierson johnpierson self-requested a review May 8, 2025 20:04
Copy link
Member

@johnpierson johnpierson left a comment

Choose a reason for hiding this comment

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

lgtm, with a couple of comments for renaming to better relate cluster with single. I would be curious @BogdanZavu 's thoughts too.

Copy link
Member

Choose a reason for hiding this comment

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

Can we rename this file to match the other one?
NodeAutoCompleteClusterResult.

So this one and the class would be NodeAutoCompleteSingleResult?

Copy link
Member

Choose a reason for hiding this comment

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

Or we could rename the other one to be simpler too, Eg. ClusterAutoCompleteResult and SingleAutoCompleteResult

Copy link
Contributor Author

@chubakueno chubakueno May 9, 2025

Choose a reason for hiding this comment

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

They have slightly different purposes, NodeAutoCompleteClusterResult is a view model for the dropdown (for both single and cluster) and SingleAutocompleteResult extracts the key necessary information from a NodeSearchElementViewModel.

So i'm renaming SingleAutocompleteResult to SingleResultItem (analog to ClusterResultItem from src\DynamoCore\Search\NodeAutocompleteSearch.cs) and NodeAutoCompleteClusterResult to DNADropdownViewModel

@johnpierson johnpierson requested review from autodesk-github-bot and removed request for autodesk-github-bot May 9, 2025 11:53
@chubakueno chubakueno merged commit de3dd4c into DynamoDS:master May 10, 2025
27 of 28 checks passed
@johnpierson
Copy link
Member

johnpierson commented May 10, 2025 via email

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.

2 participants