Skip to content

Conversation

@sheep-q
Copy link
Contributor

@sheep-q sheep-q commented Mar 6, 2025

Move ChangeBarsVisibilityModifier into ViewFactory for better customization

🔗 Issue Link

N/A

🎯 Goal

The current code work well with SwiftUI only, but if the UITabbar is implemented using UIKit, it won't function correctly.
Therefore, I'm requesting a feature to allow customizing the ChangeBarsVisibilityModifier through ViewFactory

🛠 Implementation

Move ChangeBarsVisibilityModifier logic into ViewFactory for better customization

🧪 Testing

Describe the steps how this change can be tested (or why it can't be tested).

🎨 Changes

Add relevant screenshots or videos showcasing the changes.

☑️ Checklist

  • I have signed the Stream CLA (required)
  • Changelog is updated with client-facing changes
  • New code is covered by unit tests
  • Affected documentation updated (docusaurus, tutorial, CMS (task created)

@sheep-q sheep-q requested a review from a team as a code owner March 6, 2025 02:50
/// Creates a view modifier that changes the visibility of bars.
/// - Parameter shouldShow: A Boolean value indicating whether the bars should be shown.
/// - Returns: A view modifier that changes the visibility of bars.
func changeBarsVisibility(shouldShow: Bool) -> ChangeBarsVisibilityModifier
Copy link
Contributor

Choose a reason for hiding this comment

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

Please use the same naming pattern e.g. makeChannelBarsVisibilityViewModifier

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, I've pushed a new commit to fix this, pls check it out

@sheep-q sheep-q force-pushed the chat/change_bar_visibility branch from 03b8925 to 03a8a7d Compare March 7, 2025 02:42
Copy link
Member

@nuno-vieira nuno-vieira left a comment

Choose a reason for hiding this comment

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

LGTM! ✅

@laevandus
Copy link
Contributor

@sheep-q Thank you for your contribution. We'll run CI checks manually and will merge it then.

@laevandus
Copy link
Contributor

CI checks were all good. Merging

@laevandus laevandus merged commit 25775f3 into GetStream:develop Mar 11, 2025
5 of 9 checks passed
@Stream-SDK-Bot Stream-SDK-Bot mentioned this pull request Mar 14, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants