Ankit Gade
Ankit Gade
Thanks for your valuable suggestion on E2E @eason9487. I have addressed these changes and left response to your comments. Assigning this to you for another pass.
Thanks for your review and valuable suggestions @eason9487 . I've made the changes and merging the PR.
Thanks for the IB review @nfmohit . I've made the suggested changes in IB. Assigning back to you for review.
Sorry @nfmohit , I was initially bit confused with both constants, but I've made it clear in IB now that one is for notification ID and other is for UI...
Hi @kuasha420 , If you haven't started with this one, just a heads up that `UI_KEY_SHOW_RRM_PUBLICATION_APPROVED_NOTIFICATION` constant has been added in #8796 [here](https://github.com/google/site-kit-wp/pull/9017/files#diff-6e4b84edad8f32360df76755cf6975eb960b392b16c2ac7a9e3c5e7dcb75c8a1) Thanks
I've asked Sigal to help us with the SVG issue. All other comments have been addressed. Keeping this ticket here till we hear back from Sigal about SVG.
Thank you @nfmohit for reviewing the IB. I have made the changes as per suggestions. Assigning back to you for re-review. Thanks
Thank you @nfmohit. I have updated IB. Moved the component reference to the first point and removed unnecessary styling instructions as those can be referred from the `AdsModuleSetupCTAWidget` component.
@kelvinballoo >CLARIFICATION 1: This is expected behaviour and it is same for all other CTA widget components. I believe, the banner was not dismissed previously in this case. > CLARIFICATION...
@techanvil There was a mistake in previous PR for the condition to display the component. I've created a follow up PR https://github.com/google/site-kit-wp/pull/9630 and assigning it to you for review. Thanks