-
Notifications
You must be signed in to change notification settings - Fork 293
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
Implement the secondary user variants of the "no audiences" banner. #9307
Implement the secondary user variants of the "no audiences" banner. #9307
Conversation
Build files for 1c8417a have been deleted. |
Size Change: +13 B (0%) Total Size: 1.8 MB
ℹ️ View Unchanged
|
fbb99d8
to
620f971
Compare
Set it to true when `configuredAudiences` is first set to a non-empty array.
…scenarios. The unreachable scenario being the case where there are no available audiences, as there will always be at least one, due to the "All Users" audience being un-archivable.
It will be ignored, but it's ok to pass it in as it simplifies settings updates for the client.
ae40d70
to
51a3ec1
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@techanvil Thank you for the excellent work here. Initially I had some confusions about introducing the new didSetAudiences
instead of using configuredAudiences !== null
but looking at it closely, it is really necessary.
All the other changes and deviations from IB makes sense considering the updated AC.
I merged in develop to fix some merge conflicts. The VRT failures are known issues.
With all that being said, this is LGTM. 🎉
Summary
Addresses issue:
Relevant technical choices
PR Author Checklist
Do not alter or remove anything below. The following sections will be managed by moderators only.
Code Reviewer Checklist
Merge Reviewer Checklist