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

Remove default padding in compose wrap button #321

Merged
merged 3 commits into from
Nov 30, 2023
Merged

Conversation

DevPabloGarcia
Copy link
Contributor

@DevPabloGarcia DevPabloGarcia commented Nov 29, 2023

🥅 What's the goal?

Remove default padding in compose wrap button (button2.Button)

When we use button2.Button in xml, it has a default padding. We should add the option to remove this default padding

🚧 How do we do it?

  • Add a attr to set default padding

☑️ Checks

  • I updated the documentation, including readmes and wikis. If this is a breaking change, update UPGRADING.md to inform users how to proceed. If no updates are necessary, indicate so.
  • Tested with dark mode.
  • Tested with API 23.

🧪 How can I test this?

No tests

Captura de pantalla 2023-11-29 a las 17 00 30

Captura de pantalla 2023-11-29 a las 17 01 02

Copy link

📱 New catalog for testing generated: Download

Copy link

📱 New catalog for testing generated: Download

@DevPabloGarcia DevPabloGarcia requested review from a team, jeslat and jeprubio and removed request for a team November 30, 2023 08:14
@@ -26,6 +26,7 @@ class Button @JvmOverloads constructor(
private var style: ButtonStyle by mutableStateOf(ButtonStyle.PRIMARY)
private var isEnable: Boolean by mutableStateOf(true)
private var onClick: () -> Unit by mutableStateOf({})
private var invalidatePadding: Boolean by mutableStateOf(false)
Copy link
Contributor

Choose a reason for hiding this comment

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

I would rename all to invalidatePaddings just to maintain the same name that we use in compose

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!

Copy link
Contributor

@jeprubio jeprubio left a comment

Choose a reason for hiding this comment

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

Nice

Copy link

📱 New catalog for testing generated: Download

Copy link
Contributor

@jeslat jeslat left a comment

Choose a reason for hiding this comment

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

👍

@DevPabloGarcia DevPabloGarcia merged commit cdb3f61 into main Nov 30, 2023
5 checks passed
@DevPabloGarcia DevPabloGarcia deleted the ANDROID-14050 branch November 30, 2023 08:43
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