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

Clean up AutoPromptManager vars to be class attributes #3501

Conversation

justinchou-google
Copy link
Contributor

b/303489420

@justinchou-google justinchou-google self-assigned this May 1, 2024
@justinchou-google justinchou-google changed the title clean up autoprompttype and isclosable to be class attributes Clean up AutoPromptManager vars to be class attributes May 1, 2024
@justinchou-google justinchou-google marked this pull request as ready for review May 1, 2024 21:29
Copy link
Contributor

@oyj9109 oyj9109 left a comment

Choose a reason for hiding this comment

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

(No action needed, just leaving a note)
I think ideally, isClosable should be treated as a prompt-level attribute. This change seems fine as we're not going to show multiple prompts within a page.

It might make more sense to rename the attribute to isLocked to indicate the content type of the page. We can think about making this change in the upcoming launch where we support per-prompt dismissibility, to distinguish isClosable attribute at prompt level and isLocked attribute at the page level (which is identical to the class instance).

@justinchou-google justinchou-google merged commit 5861eee into subscriptions-project:main May 2, 2024
7 checks passed
@justinchou-google justinchou-google deleted the autoprompttype branch May 2, 2024 17:08
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