Skip to content

Conversation

@nikola-milicevic
Copy link
Contributor

@nikola-milicevic nikola-milicevic commented Sep 16, 2021

Part of #15651

Description

This PR fixes the Followed sites list in Reader > Following > Filter.

Screenshots

Web version Current behaviour Desired Behaviour
Screenshot 2021-09-16 at 17 53 31 Screenshot 2021-09-16 at 17 50 35 Screenshot 2021-09-16 at 17 52 01

To test:

  1. Make sure you follow JP, WP and a non-JP, non-WP sites.
  2. List of the followed sites in the web version and the App version in Reader > Following > Filter should match.

Regression Notes

  1. Potential unintended areas of impact
    Not sure.

  2. What I did to test those areas of impact (or what existing automated tests I relied on)
    n/a

  3. What automated tests I added (or what prevented me from doing so)
    n/a

PR submission checklist:

  • I have completed the Regression Notes.
  • I have considered adding unit tests for my changes.
  • I have considered adding accessibility improvements for my changes.
  • I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

@peril-wordpress-mobile
Copy link

You can trigger an installable build for these changes by visiting CircleCI here.

@peril-wordpress-mobile
Copy link

Warnings
⚠️ This PR is assigned to a milestone which is closing in less than 4 days Please, make sure to get it merged by then or assign it to a later expiring milestone

Generated by 🚫 dangerJS

@nikola-milicevic
Copy link
Contributor Author

nikola-milicevic commented Sep 16, 2021

👋 @ScoutHarris I'm tagging you here since you've worked on the issue. It would be great if you could take a look at this and point out any potential unintended areas of impact, since I'm not aware of any, and it was all good in my testing environment.

@peril-wordpress-mobile
Copy link

You can trigger optional UI/connected tests for these changes by visiting CircleCI here.

Copy link
Contributor

@momo-ozawa momo-ozawa left a comment

Choose a reason for hiding this comment

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

Works as described! I'll defer approval to @ScoutHarris, just in case there's any unintended areas of impact 🙏

@ScoutHarris
Copy link
Contributor

Sorry I didn't get to this yesterday!

Not sure the history of excluding external sites before, but I see no reason not to now. So 👍 from me.

(Also FYI - the UI tests will be fixed with this change.)

@ScoutHarris ScoutHarris merged commit 7dd0ba2 into develop Sep 17, 2021
@ScoutHarris ScoutHarris deleted the fix/15651-reader-following-filter-list branch September 17, 2021 17:22
@ScoutHarris
Copy link
Contributor

@nikola-milicevic I went ahead and merged this so it would make the release.

@nikola-milicevic
Copy link
Contributor Author

@ScoutHarris Great, thanks! 😊

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants