Skip to content

Conversation

@johnpierson
Copy link
Member

@johnpierson johnpierson commented Jul 10, 2025

Purpose

This PR replaces hardcoded “no suggestions” placeholders with dynamic ML-based titles and messages, introduces a user authentication flag for controlling placeholder visibility, and cleans up obsolete resources and minor documentation whitespace.

  • Swap out BoxPlaceholder/NoItemsPlaceholder for TitlePlaceholder/MessagePlaceholder with bindings and a DataTrigger for DisplayAutocompleteMLStaticPage
  • Add IsUserAuthenticated property in the ViewModel to gate ML content based on login state
  • Remove old resource entries and update core resource strings for consistency
  • Clean up trailing blanks in the Node Autocomplete documentation HTML
image image

Declarations

Check these if you believe they are true

  • 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
  • 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
  • This PR introduces new feature code involve network connecting and is tested with no-network mode.

Release Notes

(FILL ME IN) Brief description of the fix / enhancement. Use N/A to indicate that the changes in this pull request do not apply to Release Notes. Mandatory section

Reviewers

@DynamoDS/synapse

FYIs

@Jingyi-Wen

@johnpierson johnpierson changed the title Dyn8778 alert user to login DYN-8778 - alert user to login Jul 10, 2025
@johnpierson johnpierson marked this pull request as ready for review July 10, 2025 22:19
Copilot AI review requested due to automatic review settings July 10, 2025 22:19
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-8778

Copy link
Contributor

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 replaces hardcoded “no suggestions” placeholders with dynamic ML-based titles and messages, introduces a user authentication flag for controlling placeholder visibility, and cleans up obsolete resources and minor documentation whitespace.

  • Swap out BoxPlaceholder/NoItemsPlaceholder for TitlePlaceholder/MessagePlaceholder with bindings and a DataTrigger for DisplayAutocompleteMLStaticPage
  • Add IsUserAuthenticated property in the ViewModel to gate ML content based on login state
  • Remove old resource entries and update core resource strings for consistency
  • Clean up trailing blanks in the Node Autocomplete documentation HTML

Reviewed Changes

Copilot reviewed 8 out of 9 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
src/NodeAutoCompleteViewExtension/Views/NodeAutoCompleteBarView.xaml Update placeholders to bind to ML properties and add DataTrigger
src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs Add IsUserAuthenticated and initial RaisePropertyChanged call
src/DynamoCoreWpf/PublicAPI.Unshipped.txt Remove obsolete AutocompleteNoSuggestions and AutocompletePlaceholder getters
src/DynamoCoreWpf/Properties/Resources.resx & Resources.en-US.resx Remove old placeholder entries
src/DynamoCore/Properties/Resources.resx & Resources.en-US.resx Change "No recommendations" title/message to new suggestion text
src/DynamoCoreWpf/Controls/Docs/en-US/NodeAutocompleteDocumentation.html Whitespace cleanup only
Files not reviewed (1)
  • src/DynamoCoreWpf/Properties/Resources.Designer.cs: Language not supported
Comments suppressed due to low confidence (4)

src/NodeAutoCompleteViewExtension/ViewModels/NodeAutoCompleteBarViewModel.cs:408

  • There are no unit tests covering the new IsUserAuthenticated logic. Add tests to verify both authenticated and unauthenticated cases.
        internal bool IsUserAuthenticated

src/DynamoCoreWpf/Controls/Docs/en-US/NodeAutocompleteDocumentation.html:1

  • The documentation hasn't been updated to describe the new ML static page behavior or login alert. Consider adding a section explaining DisplayAutocompleteMLStaticPage and the new title/message placeholders.
<!--

src/DynamoCoreWpf/PublicAPI.Unshipped.txt:4549

  • Removing these public resource getters is a breaking change. Ensure the API Changes document is updated to note this removal under Semantic Versioning guidelines.
static Dynamo.Wpf.Properties.Resources.AddStyleButton.get -> string

src/NodeAutoCompleteViewExtension/Views/NodeAutoCompleteBarView.xaml:10

  • The properties XML namespace is added but never used in this XAML. Consider removing it to reduce clutter.
             xmlns:properties="clr-namespace:Dynamo.Properties;assembly=DynamoCore"

@johnpierson johnpierson merged commit 740423e into DynamoDS:master Jul 11, 2025
24 of 25 checks passed
@johnpierson johnpierson deleted the dyn8778-AlertUserToLogin branch July 11, 2025 17:14
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