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

Sync authentication with IDSDK login/logout events #14653

Merged
merged 26 commits into from
Nov 30, 2023

Conversation

zeusongit
Copy link
Contributor

@zeusongit zeusongit commented Nov 29, 2023

Purpose

This PR adds event listeners for login/logout IDSDK methods to update shortcut toolbar and splash screen auth status.

Revit_7LYQYbEza6

Revit_12vomA0eAx

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

  • adds event listeners for login/logout IDSDK methods

Reviewers

@QilongTang
@reddyashish

@mjkkirschner
Copy link
Member

Can you add tests for this issue?

@zeusongit
Copy link
Contributor Author

Can you add tests for this issue?

Do we have IDSDK setup for test environment?

@QilongTang
Copy link
Contributor

Do you plan to review and merge #14580 as well?

@zeusongit
Copy link
Contributor Author

Do you plan to review and merge #14580 as well?

I don't know from the comments that PR still feel a bit confusing. I can take a look, but will need more clarity on what we are settling for, do we provide an API to integrators, or handle it ourselves

@QilongTang
Copy link
Contributor

Do you plan to review and merge #14580 as well?

I don't know from the comments that PR still feel a bit confusing. I can take a look, but will need more clarity on what we are settling for, do we provide an API to integrators, or handle it ourselves

I think as long as it is something integrators can use when starting splashscreen

@zeusongit
Copy link
Contributor Author

zeusongit commented Nov 29, 2023

Do you plan to review and merge #14580 as well?

I don't know from the comments that PR still feel a bit confusing. I can take a look, but will need more clarity on what we are settling for, do we provide an API to integrators, or handle it ourselves

I think as long as it is something integrators can use when starting splashscreen

Ok, I have merged his changes in this PR itself, so it will be covered.
We still need this PR DynamoDS/SplashScreen#46 on splash screen to be merged, before the Dynamo one

@QilongTang
Copy link
Contributor

Do you plan to review and merge #14580 as well?

I don't know from the comments that PR still feel a bit confusing. I can take a look, but will need more clarity on what we are settling for, do we provide an API to integrators, or handle it ourselves

I think as long as it is something integrators can use when starting splashscreen

Ok, I have merged his changes in this PR itself, so it will be covered. We still need this PR DynamoDS/SplashScreen#46 on splash screen to be merged, before the Dynamo one

Done

@zeusongit zeusongit merged commit 626878d into DynamoDS:master Nov 30, 2023
21 checks passed
@QilongTang QilongTang mentioned this pull request Dec 8, 2023
9 tasks
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.

3 participants