[Compound] Implement components (Button) #1021

jmartinesp · 2023-08-02T14:46:37Z

Changes

Implemented Button designs from Figma.
Unified all 3 button types in a single Button composable, using ButtonStyle and ButtonSize to customise its size and style (filled, outlined, text).
Merged ButtonWithProgress with this implementation too.
Replaced all button usages in all components.

Note: there will be some changes in screenshots, especially about TextButtons with different sizes and paddings. I asked @janogarcia about this and it's expected.

Closes element-hq/compound#199

github-actions · 2023-08-02T15:02:14Z

📱 Scan the QR code below to install the build (arm64 only) for this PR.

If you can't scan the QR code you can install the build via this link: https://i.diawi.com/73F3Wg

codecov · 2023-08-03T16:39:19Z

Codecov Report

Patch coverage: 76.75% and project coverage change: +0.15% 🎉

Comparison is base (c535795) 56.90% compared to head (e0aa9c1) 57.06%.
Report is 4 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #1021      +/-   ##
===========================================
+ Coverage    56.90%   57.06%   +0.15%     
===========================================
  Files          994      991       -3     
  Lines        25291    25271      -20     
  Branches      5121     5103      -18     
===========================================
+ Hits         14393    14421      +28     
+ Misses        8624     8604      -20     
+ Partials      2274     2246      -28

Files Changed	Coverage Δ
...android/features/invitelist/impl/InviteListView.kt	`58.02% <ø> (-0.52%)`	⬇️
...ogin/impl/dialogs/SlidingSyncNotSupportedDialog.kt	`0.00% <ø> (ø)`
...ent/android/features/messages/impl/MessagesView.kt	`59.16% <ø> (-0.22%)`	⬇️
...impl/attachments/preview/AttachmentsPreviewView.kt	`58.66% <0.00%> (+0.77%)`	⬆️
...ignsystem/atomic/molecules/ButtonColumnMolecule.kt	`53.84% <ø> (ø)`
...designsystem/atomic/molecules/ButtonRowMolecule.kt	`45.45% <ø> (ø)`
...ignsystem/components/dialogs/ConfirmationDialog.kt	`62.90% <ø> (+1.36%)`	⬆️
...android/libraries/designsystem/ruler/WithRulers.kt	`65.21% <ø> (ø)`
...eatures/createroom/impl/addpeople/AddPeopleView.kt	`43.18% <33.33%> (-1.72%)`	⬇️
...nfirmaccountprovider/ConfirmAccountProviderView.kt	`36.00% <33.33%> (+0.86%)`	⬆️
... and 20 more

... and 2 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

jmartinesp · 2023-08-08T06:46:57Z

I decided to create a single Button component with different ButtonStyles, but if you think some FilledButton/OutlineButton/TextButton helper functions would be useful let me know and I'll add them.

julioromano · 2023-08-08T06:52:23Z

I decided to create a single Button component with different ButtonStyles, but if you think some FilledButton/OutlineButton/TextButton helper functions would be useful and I'll add them.

I'd very much like it, sounds very idiomatic.
Since the types are very similar to material's we could still mimic their names:
Button (filled)
OutlinedButton (not filled)
TextButton (no fill, no border)

jmartinesp · 2023-08-08T10:20:54Z

@julioromano sorry for the back and forth, it should be ready for review now

julioromano

Some comments to refine the API

julioromano · 2023-08-08T08:55:15Z

...nsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt

 import androidx.compose.ui.tooling.preview.Preview
 import androidx.compose.ui.unit.dp
 import io.element.android.libraries.designsystem.preview.ElementThemedPreview
 import io.element.android.libraries.designsystem.preview.PreviewGroup
+import io.element.android.libraries.theme.ElementTheme
+
+// Designs: https://www.figma.com/file/G1xy0HDZKJf5TCRFmKb5d5/Compound-Android-Components?type=design&mode=design&t=U03tOFZz5FSLVUMa-1

 @Composable
 fun Button(


Perhaps this can become a private composable that the 3 public one use?
I've seen this pattern in use throughout the compose codebase if I'm not mistaken.

Maybe named "InnerButton" or "ButtonContent"

julioromano · 2023-08-08T13:57:06Z

...nsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt


 @Composable
 fun Button(
+    title: String,


I'd call this "text" because title seems to assume a style or that there will be other text elements (e.g. a description or subtitle).

julioromano · 2023-08-08T13:59:09Z

...nsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt

-    contentPadding: PaddingValues = ElementButtonDefaults.ContentPadding,
-    interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
-    content: @Composable RowScope.() -> Unit
+    buttonSize: ButtonSize = ButtonSize.Large,


Prepending these params with "button" seem redundant given the context.

I'd simply go with "size" and "style".

julioromano · 2023-08-08T13:59:58Z

...nsystem/src/main/kotlin/io/element/android/libraries/designsystem/theme/components/Button.kt

-    interactionSource: MutableInteractionSource = remember { MutableInteractionSource() },
-    content: @Composable RowScope.() -> Unit
+    buttonSize: ButtonSize = ButtonSize.Large,
+    buttonStyle: ButtonStyle = ButtonStyle.Filled,


This should be removed from the public API surface in favor of using OutlinedButton and TextButton

julioromano

Proactive LGTM. Please have a look at the comments though.

- Make `ButtonStyle` private. - Rename `title` to `text`. - Rename `buttonStyle` and `buttonSize` to just `style` and `size`.

sonarcloud · 2023-08-08T16:11:14Z

Kudos, SonarCloud Quality Gate passed!

0 Bugs
0 Vulnerabilities
0 Security Hotspots
2 Code Smells

0.0% Coverage
0.0% Duplication

jmartinesp added 6 commits August 8, 2023 07:48

Create CompoundButton

1d10a40

Some fixes

15a39c7

Lint fixes

70bf0c8

Start replacing existing Button usages

13b6586

Replace button usages

398a9c3

Remove previous Button composable

7125e59

jmartinesp force-pushed the chore/jme/compound-platform-components-button branch from 5e488bb to 86d592f Compare August 8, 2023 05:51

jmartinesp added 3 commits August 8, 2023 07:59

Rename CompoundButton to Button

e1353e9

Fix emphasized button being displayed as Text

cda6b02

Fix cancel button in WaitListView

531c6ff

jmartinesp force-pushed the chore/jme/compound-platform-components-button branch from 86d592f to 531c6ff Compare August 8, 2023 05:59

Update screenshots

9c1c0eb

jmartinesp marked this pull request as ready for review August 8, 2023 06:46

jmartinesp requested a review from a team as a code owner August 8, 2023 06:47

jmartinesp requested review from bmarty, a team and julioromano and removed request for a team and bmarty August 8, 2023 06:47

jmartinesp added 2 commits August 8, 2023 09:13

Add shorthand functions for OutlinedButton and TextButton

37b66db

Add changelog

41a9a9b

jmartinesp force-pushed the chore/jme/compound-platform-components-button branch from 1254214 to 0797d98 Compare August 8, 2023 09:36

Fix wrong size used for emphasized button in dialog

5e89ed3

jmartinesp force-pushed the chore/jme/compound-platform-components-button branch from f71de7c to 5e89ed3 Compare August 8, 2023 10:06

julioromano reviewed Aug 8, 2023

View reviewed changes

julioromano approved these changes Aug 8, 2023

View reviewed changes

jmartinesp added 2 commits August 8, 2023 16:49

Create a private ButtonInternal implementation with the shared logic.

b917292

- Make `ButtonStyle` private. - Rename `title` to `text`. - Rename `buttonStyle` and `buttonSize` to just `style` and `size`.

Fix several warnings and lint issues.

0ac6836

jmartinesp enabled auto-merge August 8, 2023 15:36

Update screenshots

e0aa9c1

jmartinesp added this pull request to the merge queue Aug 8, 2023

Merged via the queue into develop with commit 23982dd Aug 8, 2023

jmartinesp deleted the chore/jme/compound-platform-components-button branch August 8, 2023 16:12

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Compound] Implement components (Button) #1021

[Compound] Implement components (Button) #1021

jmartinesp commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023 •

edited

Loading

codecov bot commented Aug 3, 2023 •

edited

Loading

jmartinesp commented Aug 8, 2023 •

edited

Loading

julioromano commented Aug 8, 2023

jmartinesp commented Aug 8, 2023

julioromano left a comment

julioromano Aug 8, 2023

julioromano Aug 8, 2023

julioromano Aug 8, 2023

julioromano Aug 8, 2023

julioromano left a comment •

edited

Loading

sonarcloud bot commented Aug 8, 2023

[Compound] Implement components (Button) #1021

[Compound] Implement components (Button) #1021

Conversation

jmartinesp commented Aug 2, 2023 • edited Loading

Changes

github-actions bot commented Aug 2, 2023 • edited Loading

codecov bot commented Aug 3, 2023 • edited Loading

Codecov Report

jmartinesp commented Aug 8, 2023 • edited Loading

julioromano commented Aug 8, 2023

jmartinesp commented Aug 8, 2023

julioromano left a comment

Choose a reason for hiding this comment

julioromano Aug 8, 2023

Choose a reason for hiding this comment

julioromano Aug 8, 2023

Choose a reason for hiding this comment

julioromano Aug 8, 2023

Choose a reason for hiding this comment

julioromano Aug 8, 2023

Choose a reason for hiding this comment

julioromano left a comment • edited Loading

Choose a reason for hiding this comment

sonarcloud bot commented Aug 8, 2023

jmartinesp commented Aug 2, 2023 •

edited

Loading

github-actions bot commented Aug 2, 2023 •

edited

Loading

codecov bot commented Aug 3, 2023 •

edited

Loading

jmartinesp commented Aug 8, 2023 •

edited

Loading

julioromano left a comment •

edited

Loading