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

Conversation

@hichamboushaba
Copy link
Member

@hichamboushaba hichamboushaba commented Dec 27, 2024

Needed for woocommerce/woomobile-private#385

This PR adds the following changes:

  • Removes an outdated logic of blur that causes blur to be shown even when not configured (more details in the comments section).
  • Update logic to set both the background color and image at the same time.
  • Allow setting a scaling mode for the background image.
  • Allow customizing the style of the tertiary button in the prologue.
  • Fix an issue with the screen sizing (more details in the comments section).

To be tested with woocommerce/woocommerce-ios#14773


  • I have considered if this change warrants release notes and have added them to the appropriate section in the CHANGELOG.md if necessary.

Comment on lines +153 to +154
topStackView?.isHidden = true
bottomStackView?.isHidden = true
Copy link
Member Author

@hichamboushaba hichamboushaba Dec 31, 2024

Choose a reason for hiding this comment

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

Before this, both stack views were shown even when they are empty, and for some reason this causes the button section to take more space than needed when the top layout doesn't dictate a specific sizing.
I'm not entirely sure about the cause here, but my basic understanding is that when the stackview is empty and shown, it doesn't report an intrinsic size, so AutoLayout doesn't know how much space to give it when the top view doesn't report a size either.

Anyway, regardless of the explanation, I think hiding the empty view is what we should do, and given this fixes the issue, it's just better.

(I tried setting a higher priority for the content hugging constraint for the button container, but it didn't have an effect).

Comment on lines -401 to -404
guard prologueViewBackgroundColor.cgColor == buttonsBackgroundColor.cgColor else {
buttonBlurEffectView.effect = UIBlurEffect(style: blurEffect)
return
}
Copy link
Member Author

Choose a reason for hiding this comment

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

This logic is old, we had it when we didn't allow client apps to control the blur effect, later, this ability was added. Also, this old logic was causing me an issue, since with the new design, we need to have the buttons background as .clear, which means the condition applies here, and it causes the blur to be shown even when it's not configured, removing the logic fixes the issue.

Copy link
Contributor

@itsmeichigo itsmeichigo left a comment

Choose a reason for hiding this comment

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

Tested as part of woocommerce/woocommerce-ios#14773 and confirmed that the new changes work correctly in iPhone SE, iPhone 16 Plus and iPad Mini simulators ✅.

@hichamboushaba hichamboushaba merged commit e5247b2 into trunk Jan 2, 2025
11 checks passed
@hichamboushaba hichamboushaba deleted the wcios/prologue-customization branch January 2, 2025 09:21
mokagio added a commit to woocommerce/woocommerce-ios that referenced this pull request Jan 6, 2025
When wordpress-mobile/WordPressAuthenticator-iOS#867
was merged, I had already started working on folding
WordPressAuthenticator and didn't sync with the team about it.

This commit picks all the changes from that PR and copies them in one go
into the local version of the WordPressAuthenticator codebase.
mokagio added a commit to woocommerce/woocommerce-ios that referenced this pull request Jan 9, 2025
When wordpress-mobile/WordPressAuthenticator-iOS#867
was merged, I had already started working on folding
WordPressAuthenticator and didn't sync with the team about it.

This commit picks all the changes from that PR and copies them in one go
into the local version of the WordPressAuthenticator codebase.
mokagio added a commit to woocommerce/woocommerce-ios that referenced this pull request Jan 16, 2025
When wordpress-mobile/WordPressAuthenticator-iOS#867
was merged, I had already started working on folding
WordPressAuthenticator and didn't sync with the team about it.

This commit picks all the changes from that PR and copies them in one go
into the local version of the WordPressAuthenticator codebase.
mokagio added a commit to woocommerce/woocommerce-ios that referenced this pull request Jan 20, 2025
When wordpress-mobile/WordPressAuthenticator-iOS#867
was merged, I had already started working on folding
WordPressAuthenticator and didn't sync with the team about it.

This commit picks all the changes from that PR and copies them in one go
into the local version of the WordPressAuthenticator codebase.
@toupper toupper mentioned this pull request Jan 31, 2025
1 task
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants