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(button): add icon alone UIKit VitaminButton type, and refactor icon management #75

Merged
merged 5 commits into from
Jul 26, 2022

Conversation

daniel-dumortier
Copy link
Contributor

@daniel-dumortier daniel-dumortier commented Jul 22, 2022

Changes description

I added the ability to define a 'icon Alone' version of VitaminButton.
I also reworked the way to specify which type of icon you want (for already existing trailing and leading icons)
And I took advantage of this PR to change icon size on large and medium button (see #74)

Context

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 with a core team designer.

Does this introduce a breaking change?

  • No
    I deprecated some methods, but they are still usable, the will just raise a warning.

Screenshots

iPhone

Simulator Screen Shot - iPhone 8 - 2022-07-22 at 17 43 28
Simulator Screen Shot - iPhone 8 - 2022-07-22 at 17 43 47

iPad

Simulator Screen Shot - iPad (9th generation) - 2022-07-22 at 17 55 30
Simulator Screen Shot - iPad (9th generation) - 2022-07-22 at 17 56 01

Other information

I am not sure it is Vitamin compliant, but I reworked the code to give the ability to have different icons for different states (that was allowed before by the previous code, but did not work well)

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.

Good work!
I have just a few minor suggestions. I let you check.

Sources/VitaminUIKit/Components/Button/README.md Outdated Show resolved Hide resolved
Sources/VitaminUIKit/Components/Button/README.md Outdated Show resolved Hide resolved
Sources/VitaminUIKit/Components/Button/VitaminButton.swift Outdated Show resolved Hide resolved
@daniel-dumortier daniel-dumortier changed the title feat(button) : add icon alone UIKit VitaminButton type, and refactor icon management feat(button): add icon alone UIKit VitaminButton type, and refactor icon management Jul 25, 2022
@daniel-dumortier daniel-dumortier added the Ready for Design Review The showcase is ready for design review label Jul 25, 2022
Copy link
Contributor

@baptistedajon baptistedajon left a comment

Choose a reason for hiding this comment

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

👍🏻

@Sabrinavigil
Copy link

👍

@sonarqubecloud
Copy link

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

@daniel-dumortier daniel-dumortier removed the Ready for Design Review The showcase is ready for design review label Jul 26, 2022
@daniel-dumortier
Copy link
Contributor Author

Some problems have been detected on existing buttons, but since they are already released, I just opened a new issue #77 and merge this feature.

@daniel-dumortier daniel-dumortier merged commit fd92229 into main Jul 26, 2022
@daniel-dumortier daniel-dumortier deleted the feat/button_icon_alone branch July 26, 2022 15:30
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.

bug: Icon size for button feat: add button icon alone on iOS
4 participants