Skip to content
This repository was archived by the owner on Feb 5, 2025. It is now read-only.

Conversation

@ctarda
Copy link
Contributor

@ctarda ctarda commented Dec 13, 2020

Closes #543
Ref: woocommerce/woocommerce-ios#3309
Ref: woocommerce/woocommerce-ios#3330
PR in WooCommerce: woocommerce/woocommerce-ios#3339

The way I implemented the removal of the TOS was by just removing the cell from the table view. That makes the bottom hairline in the email textfield cell to sit right above the top margin of the Continue button.

I considered just hiding the TOS button instead of removing the cell completely (you can see that if you look at this very PR history) but in the end I thought a Spacer cell (a completely empty one) might be a better way to handle this.

Changes

  • Add a spacer cell
  • Declare a new section in GetStartedViewController to model the spacer
  • Check if the host app wants to display TOS or not. If it doesn't, add a spacer.

How to test

  • Look at the code. Does the change make sense?
  • Check that the corresponding PRs in WooCommerce and WordPress work as expected.

@ctarda ctarda self-assigned this Dec 13, 2020
@ctarda ctarda added the enhancement New feature or request label Dec 13, 2020
@ctarda ctarda marked this pull request as draft December 13, 2020 23:45
@ctarda ctarda marked this pull request as ready for review December 14, 2020 01:28
Copy link
Contributor

@mindgraffiti mindgraffiti left a comment

Choose a reason for hiding this comment

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

I think you will want to un-check User Interaction Enabled for the spacer cell, otherwise VoiceOver will attempt to identify it. I'm unable to test and confirm this claim because my device isn't registered for Woo use.

Screen Shot 2020-12-14 at 10 03 26 AM

Other than that, :shipit: !

Pod::Spec.new do |s|
s.name = "WordPressAuthenticator"
s.version = "1.31.0-beta.5"
s.version = "1.31.0-beta.6"
Copy link
Contributor

Choose a reason for hiding this comment

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

Since the new release was cut today, this should probably be 1.32.0-beta.1 after @jkmassel finishes the releases.

@ctarda
Copy link
Contributor Author

ctarda commented Dec 15, 2020

Thanks @mindgraffiti @ScoutHarris I'll merge develop into the branch, un-check userInteractionEnabled, update the podspec and merge 😊

@ctarda ctarda merged commit a22b96f into develop Dec 15, 2020
@ctarda ctarda deleted the issue/543-tos-padding branch December 15, 2020 02:17
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add padding between email and continue button when TOS is hidden

4 participants