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

IOS-10800 Make ViewStates component in SwiftUI #415

Merged
merged 3 commits into from
Dec 16, 2024

Conversation

alejandroruizponce
Copy link
Contributor

@alejandroruizponce alejandroruizponce commented Nov 28, 2024

🎟️ Jira ticket

https://jira.tid.es/browse/IOS-10800

πŸ₯… What's the goal?

As a performance objective to make SwiftUI components, ViewStates component will be migrated from UIKit to SwiftUI.

🚧 How do we do it?

A new component similar to the component in UIKit has been created and added inside the Catalog.

πŸ§ͺ How can I verify this?

FOR DESIGN MEMBER (@AnaMontes11) : There is a difference between the buttons we have for SwiftUI and the UIKit buttons, I'm not clear which one should be the correct one since I can't find references to the Design of this component. I hope that in the revision of this PR a member of Design can clarify it.

If the SwiftUI design is correct, it could be used as a base and apply in UIKit a wrapper that encapsulates the SwiftUI design to use the same design in all platforms.

SwiftUI UIKit
Β Screenshot 2023-10-03 at 15 27 29Β  Screenshot 2023-10-03 at 15 30 48

πŸ‘ AppCenter build

Screenshot 2024-11-29 at 12 12 00

@alejandroruizponce alejandroruizponce requested review from DavidMarinCalleja, a team, franciscojrp and AnaMontes11 and removed request for a team November 28, 2024 16:00
@AnaMontes11
Copy link

Hi @alejandroruizponce ,
Here’s the link to the button component specifications from MΓ­stica: Buttons component Specs.
You can review it to clarify any doubts and make the necessary adjustments.
Let me know if this helps or if you need anything else. :)

Copy link
Contributor

@DavidMarinCalleja DavidMarinCalleja left a comment

Choose a reason for hiding this comment

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

There would be a missing test for this component, wouldn't there?

@alejandroruizponce
Copy link
Contributor Author

Hi @alejandroruizponce , Here’s the link to the button component specifications from MΓ­stica: Buttons component Specs. You can review it to clarify any doubts and make the necessary adjustments. Let me know if this helps or if you need anything else. :)

Thanks @AnaMontes11, but really my question is more about what type of button is correct for this View State component. If the one we use in UIKit or the SwiftUI one, or if both are valid.

@alejandroruizponce
Copy link
Contributor Author

There would be a missing test for this component, wouldn't there?

There were no tests for the component in UIKit either, if I can I will check and add some.

@AnaMontes11
Copy link

Hi @alejandroruizponce, this is not a MΓ­stica component; I understand it comes from an old View State. On our side, it doesn’t matter if you use a small or large button, although for consistency, it’s better to use the same one in both. If this decision is relevant for Novum or could affect something in production, we’d need to discuss it with the product team. Let me know if I’ve clarified your question.

@AnaMontes11
Copy link

Hello again, @alejandroruizponce As I mentioned, it seems like an older empty state. If it helps, I would use the same (large) button in the SwiftUI version for consistency and discuss with the team updating both to something closer to a more modern empty state.

@alejandroruizponce
Copy link
Contributor Author

Hello again, @alejandroruizponce As I mentioned, it seems like an older empty state. If it helps, I would use the same (large) button in the SwiftUI version for consistency and discuss with the team updating both to something closer to a more modern empty state.

Talked with @AnaMontes11 and @aweell, same large button will be used on both platforms even though they are different right now. The unification of the button can be looked at in a different ticket as well as the update of the texts:
https://jira.tid.es/browse/IOS-10849

@alejandroruizponce alejandroruizponce force-pushed the IOS-10800-Migrate-ViewStates-to-SwiftUI branch from ea6e0dc to 7eca443 Compare December 16, 2024 13:17
@alejandroruizponce alejandroruizponce merged commit c6da83a into main Dec 16, 2024
2 checks passed
@alejandroruizponce alejandroruizponce deleted the IOS-10800-Migrate-ViewStates-to-SwiftUI branch December 16, 2024 13:45
@tuentisre
Copy link
Collaborator

πŸŽ‰ This PR is included in version 33.7.0 πŸŽ‰

The release is available on GitHub release

Your semantic-release bot πŸ“¦πŸš€

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants