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

Add banner to subscription action #4312

Merged
merged 18 commits into from Sep 9, 2022
Merged

Add banner to subscription action #4312

merged 18 commits into from Sep 9, 2022

Conversation

mazevedofs
Copy link
Collaborator

Phabricator: https://phabricator.wikimedia.org/T311639

Notes

  • This task adds a banner to the subscription action
  • We are not calling the API at this moment (but methods were added to the view model)
  • The layout differs from figma, as we are using an existing component at the moment

Test Steps

  1. Go to a talk page, and click the subscribe button. Make sure the alert shows at the bottom.
  2. Click unsubscribe, verify the banner

@mazevedofs mazevedofs requested review from a team and tonisevener and removed request for a team September 9, 2022 18:10
@tonisevener tonisevener self-assigned this Sep 9, 2022
Copy link
Collaborator

@tonisevener tonisevener left a comment

Choose a reason for hiding this comment

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

Looking good! I didn't realize this view was so broken with bottom alignment and icons, thanks for working through it. I made a couple of minor tweaks in 966837e, hope that's okay:

  1. There were some console constraint complaints, and after some digging it looks like there already was a leading constraint in the xib that your new titleViewLeading constraint conflicted with. So I exposed that xib constraint and am setting it to 50 instead.
  2. I aligned some views to the superview safeAreaLayoutGuide instead of the extra safe area padding number. Both of these changes should only affect bottom and icon banners, so I think it's pretty safe.

override func viewDidLoad() {
super.viewDidLoad()

navigationItem.title = WMFLocalizedString("talk-pages-view-title", value: "Talk", comment: "Title of user and article talk pages view.")
Copy link
Collaborator

Choose a reason for hiding this comment

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

Thanks for cleaning these up!

@tonisevener tonisevener merged commit 7a396ff into main Sep 9, 2022
@tonisevener tonisevener deleted the T311639-subscription branch September 9, 2022 20:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants