Tom Rees-Herdman

Results 525 comments of Tom Rees-Herdman

Hi @aaemnnosttv, this is still an issue, yup, at least it is for PSI as evident clicking on the link in the PSI setup notification. Notice how it navigates to...

Hey @tofumatt, it turns out the PSI notification link is [already using](https://github.com/google/site-kit-wp/blob/7d6ef99d738bd6d23a0fb2fad38109d66d6fe6ab/assets/js/components/notifications/SetupSuccessBannerNotification.js#L202-L212) `behaviour: smooth` for the scrolling, so these AC need to be revised. I think that as per the...

Thanks @tofumatt. It works in the context of the conversation we've had, but I think it could be a bit ambiguous to a fresh pair of eyes. It would be...

Hey @benbowler, thanks for drafting this IB. A few points: - As per the dependency #8487, `getAudiences()` will be replaced with `getAvailableAudiences()` by the time this issue is implemented, so...

Hi @jimmymadon, thanks for drafting the IB here which LGTM. However, while we do need to allow a good amount of time for testing - the estimate still seems a...

Note that I made a small amendment to the AC to reflect the change introduced in https://github.com/google/site-kit-wp/issues/9168 which ensures the Site Kit-created audiences are always listed in the order "new...

Hi @zutigrm, thanks for drafting this IB. A minor point - as per the new [code organization](https://docs.google.com/document/d/1MGD5Djy6AeeZC4zBtHqS-lQEWD9jw0kf-IIWw-jLCFU/edit#heading=h.mlj74m7pz9gr) section in the design doc, the SASS file for the component should be...

Thanks @hussain-t, this IB is generally looking good. One thing though, as per my previous [comment](https://github.com/google/site-kit-wp/issues/8158#issuecomment-1927156487), we need to ensure that the list of available audiences takes into account the...