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

DYN-4973-Popups-NotAttached #13002

Merged
merged 8 commits into from
Jun 16, 2022
Merged

DYN-4973-Popups-NotAttached #13002

merged 8 commits into from
Jun 16, 2022

Conversation

RobertGlobant20
Copy link
Contributor

Purpose

I implemented the functionality of hiding/showing the FileTrustUI and GuidedTours popups when the Dynamo window is inactive (in background).
I've tested the functionality for the User Interface tour and Packages tour and for the exitwindow that appear in the Packages tour.
For the other tours the RealTimeWindow (that appear when exiting the Guide) wont' have this functionality implemented.

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

Release Notes

I implemented the functionality of hiding/showing the FileTrustUI and GuidedTours popups when the Dynamo window is inactive.

Reviewers

@QilongTang

FYIs

I implemented the functionality of  hiding/showing the FileTrustUI and GuidedTours popups when the Dynamo window is inactive (in background).
I've tested the functionality for the User Interface tour and Packages tour and for the exitwindow that appear in the Packages tour.
For the other tours the RealTimeWindow (that appear when exiting the Guide) wont' have this functionality implemented.
@RobertGlobant20
Copy link
Contributor Author

This GIF shows the functionality for the Popups in the Guided Tour
HidePopupFunctionality

@RobertGlobant20
Copy link
Contributor Author

This GIF shows the functionality for the Popup in the FileTrustUI
HideFileTrustPopup

@zeusongit
Copy link
Contributor

Does this also fix toast notifications, are they also attached to parent window now?

@RobertGlobant20
Copy link
Contributor Author

Does this also fix toast notifications, are they also attached to parent window now?

@zeusongit
if you are talking about the RealtimeInfoWindow (see image below), no, it's just fixing the next Popups:

  • Guided Tours
  • File Trust UI
  • Search (ContextWindow appearing when clicking right over the Workspace)

For the RealTimeInfo popup I think @QilongTang changed the StaysOpen property in a different PR then it should be closing the Popup when the focus is lost.

image

I removed all the functionality of setting a state in the Guide and also added a new event for when the tour is completely closed (just in the case of the Packages guide).
@QilongTang QilongTang added this to the 2.15.0 milestone Jun 15, 2022
Changed the hard-coded string to use the static string
Copy link
Contributor

@QilongTang QilongTang left a comment

Choose a reason for hiding this comment

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

Some comments, then LGTM

Event renamed from GuidedTourClosed to GuidedTourFinished.
The property warningPopup was renamed to fileTrustWarningPopup.
Event renamed from GuidedTourFinished to GuidedTourExited
Fixing function name to match event name.
Removed subscriptions to GuidedTourExited event and the empty function handler.
Renaming property to IsGuideExited
@QilongTang QilongTang merged commit e191d22 into DynamoDS:master Jun 16, 2022
QilongTang pushed a commit that referenced this pull request Jun 29, 2022
After the changes done in the next PR: #13002 I introduced a bug that was not allowing to save the path added in trusted locations, then with this fix is saving again the trusted locations.
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