Implement inline loading component - #129
Conversation
4fa760d to
cbb7994
Compare
| * @param interactionSource The [MutableInteractionSource] that keeps track of the button's state. | ||
| */ | ||
| @Composable | ||
| public fun InlineLoadingButton( |
There was a problem hiding this comment.
Would it be better to add an overload to Button instead of creating a separate InlineLoadingButton component? I wasn't sure which approach aligns best with the architecture.
There was a problem hiding this comment.
That's a good question. The approach I would have taken is integrating the inline loading directly into the button component, making it available with an optional inlineLoadingState: InlineLoadingState? = null. Default null value meaning no loading.
If you look at the existing Button API, you'll see that there's an alt one wrapping the component to display a tooltip. If we are to create an InlineLoadingButton, then we also need to make a tooltip alt, thus increasing the API complexity, tests and maintenance efforts.
For instance, by just adding a parameter to the existing APIs, you just need to add a coverage for it in the existing tests, instead of creating a new test class, and it would require re-writing the same test from the Button tests, or refactor them in an abstraction to be shared between the two implementations.
What do you think?
There was a problem hiding this comment.
Sure! I can defo. do that. No issues.
gabrieldrn
left a comment
gabrieldrn
left a comment
There was a problem hiding this comment.
Thank you for your contribution, and sorry for the late notice. Very clean work!
| * (From [Inline loading documentation](https://carbondesignsystem.com/components/inline-loading/usage)) | ||
| */ | ||
| @Immutable | ||
| @Suppress("UndocumentedPublicProperty") |
There was a problem hiding this comment.
Would be better to add them in Detekt's baseline, to mark them as "officially not documented". I tend to consider the Suppress annotation to mark stuff with a temporary "fix".
| ~ limitations under the License. | ||
| --> | ||
|
|
||
| <vector xmlns:android="http://schemas.android.com/apk/res/android" |
There was a problem hiding this comment.
Not sure where you found that vectorized drawing haha, but it should be the one you find at https://carbondesignsystem.com/components/overview/components/ in the components grid instead. I use an inspector in a web browser to extract the SVG asset then convert it to an AVD.
There was a problem hiding this comment.
Yeah, ngl this was the hardest part. :D
I have had to use an svg converter and then once imported as a vector asset, I had to manipulate it for it to look proper but your approach seems better. I'll update it using the link you mentioned. Thanks.
| */ | ||
| @Immutable | ||
| @Suppress("UndocumentedPublicProperty") | ||
| public enum class InlineLoadingStatus { |
There was a problem hiding this comment.
To stick with the rest of the implementations and the design system lexicon, using the State word instead of Status would be preferable.
| Box(modifier = modifier) { | ||
| when (status) { | ||
| InlineLoadingStatus.Inactive -> Unit | ||
| InlineLoadingStatus.Active -> SmallLoading() |
There was a problem hiding this comment.
Looking at the web implementations, the blue arc seems to have an angle of 60°, while SmallLoading is 270°. Feel free to refactor the loading component if it helps to have this 60° for the inline loading, but keep the existing API signatures
2 participants
Notes / Description
This PR adds the
InlineLoadingcomponent set to the sharedcarbonmodule, including both the standalone inline loading primitive and anInlineLoadingButtonvariant.Resolves
What's included
InlineLoadingcomposable incommonMainInlineLoadingStatusenum with the supported states:InactiveActiveFinishedErrorcontentDescriptionoverrideInlineLoadingButtoncomposable incommonMain:Inline loadingdemo destination/screenInlineLoadingandInlineLoadingButtonInline loadingto the components indexInline loading buttonto the components indexInline loadingas available in the component availability matrixPlatforms testing checklist
Screenshots / videos