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

feat: SegmentedControl UIKit implementation #115

Merged
merged 8 commits into from
Mar 9, 2023

Conversation

daniel-dumortier
Copy link
Contributor

@daniel-dumortier daniel-dumortier commented Feb 6, 2023

Changes description

Implementation of Vitamin tab component for UIKit.
It is a subclass of standard UISegmentedControl, with only customization of:

  • text style and color
  • shadow

The PR also contains a VitaminTabsController that handles a common use case : display a VitaminTabs at the top of the screen, and a viewController below, depending on the tab selected

Context

UIKit part of #41

Checklist

  • Make sure you are requesting to pull a topic/feature/bugfix branch. Please, don't request directly from your main!
  • Check commits & PR names matches our requested structure. It must follow the https://www.conventionalcommits.org pattern.
  • Check your code additions will fail neither code linting checks.
  • I have reviewed the submitted code.
  • I have tested on an iPhone device/simulator.
  • I have tested on an iPad device/simulator.
  • If it includes design changes, please ask for a review design-system-core-team-design GitHub team.

Does this introduce a breaking change?

  • No

Screenshots

iPhone

Simulator Screen Shot - iPhone 8 - 2023-02-06 at 23 22 55

iPad

Simulator Screen Shot - iPad (9th generation) - 2023-02-06 at 23 23 59

Other information

I named the component VitaminTab, but I wonder if it should not been named VitaminSegmentedControl, that would be more understandable by iOS developers.

@daniel-dumortier daniel-dumortier requested a review from a team February 8, 2023 13:13
@SimonLeclercq
Copy link

Hey @daniel-dumortier, Feel free to discuss with @florentlotthepro about the component name. Keep in mind that we want to stay as close as possible to the developer's experience of the platform. On the Design side, this component is named segmented control.

@SimonLeclercq
Copy link

@daniel-dumortier I'm not able to find the bitrise link 🤔 Can you help me? 😎

@daniel-dumortier
Copy link
Contributor Author

@SimonLeclercq the showcase has not been built yet, that's why you can't find the link ;)
Since I added a new feature on this branch following @florentlotthepro 's suggestion (in short, a generic screen that handles the display of other sub-screens depending on the segment selected), I wait for other code owners technical review before submitting it to you (to ensure it is clean on a code point of view)

Copy link
Contributor

@florentlotthepro florentlotthepro left a comment

Choose a reason for hiding this comment

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

Like said by Simon, what do you think about VitaminSegmentedControl? (and VitaminSegmentedController) It's not exactly like Vitamin Android but it feels more iOS. For me, Tabs feels like related to TabBar.

Sources/VitaminUIKit/Components/Tabs/README.md Outdated Show resolved Hide resolved
Sources/VitaminUIKit/Components/Tabs/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@florentlotthepro florentlotthepro 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 that it could be nice to also rename the folder for the component 😇

README.md Outdated Show resolved Hide resolved
@florentlotthepro florentlotthepro changed the title feat: tabs UIKit implementation feat: SegmentedControl UIKit implementation Feb 15, 2023
@daniel-dumortier daniel-dumortier added the Ready for Design Review The showcase is ready for design review label Feb 16, 2023
@daniel-dumortier
Copy link
Contributor Author

@SimonLeclercq The build is available, you can find the link on Slack

Copy link

@SimonLeclercq SimonLeclercq 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 I see a slight horizontal shift on the labeltext compared to the container. Can you confirm this? Besides that, it's good for me.

@daniel-dumortier
Copy link
Contributor Author

@SimonLeclercq
Honestly, I do not really get what you mean by horizontal shift, but for info, I just styled the native SegmentedControl component (with custom font, size, colors, and shadow), thus I do not have any way to shift or move the labels in the segmented control.
I just let iOS place them inside the component.

@sonarcloud
Copy link

sonarcloud bot commented Mar 8, 2023

Kudos, SonarCloud Quality Gate passed!    Quality Gate passed

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.0% 0.0% Duplication

@SimonLeclercq
Copy link

Ok so no problem 👍 thanks for feedback !

@daniel-dumortier daniel-dumortier merged commit 58e0b0e into main Mar 9, 2023
@daniel-dumortier daniel-dumortier deleted the feature/tab_uikit branch March 9, 2023 12:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement 🚀 New feature or request Ready for Design Review The showcase is ready for design review UIKit
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants