Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
Adds scaffolding for the ButtonComponent. #4348
base: main
Are you sure you want to change the base?
Adds scaffolding for the ButtonComponent. #4348
Changes from 4 commits
8324986
4200f5e
13c48c6
95fe157
857fad6
62b33f7
310bee9
26fbb79
393590a
c0f4f73
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks similar to the
LinkButton
where it has aTextComponent
inside of it (which makes sense) but I totally forgot to delete theLinkComponent
and mention when why it was build that way 😅 It was built for very early demo purpose and just reusedTextComponent
because easy.However, I think we are going to be leaning towards more of an protocol style instead of composition for components that have similar things here.
I started doing this with the
StackableComponent
in https://github.com/RevenueCat/purchases-ios/pull/4324/files#diff-ec1da30bf93d1075c4c1985847a4762e27386eb3a6cb81a81dee288d50ba2fccSo here, we could create a new protocol called like
TextableComponent
(names are hard???) that have the properties that are similar betweenTextComponent
andButtonComponent
.My reason for this is:
TextComponent
, we might not wantButtonComponent
to have it (which actually happens in [WIP] Added selected state to TextComponentView #4346 with selection state)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That makes sense! However, are we going to allow people to give buttons arbitrary content, like SwiftUI and Jetpack Compose do? What about icons/images?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is... a great question 🤔 In the current spec, the button only has text. However...
Maybe a
ButtonComponent
should really be a container/stack and hold a list of components... and then the dashboard component selector can give options for likeTextButtonComponent
orIconButtonComponent
that adds a button with those sub components in it already as a template?Does that make sense? If so... we should bring this up to the Dan and Barbara 😇
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea that does make sense! And then things like "leading icon" vs "trailing icon" are just the same (horizontal) stack, but in a different order. I'll bring it up!
Yea, or it could have a single
StackComponent
child?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe making buttons behave like stacks (in whatever implementation) might be good to do regardless, to not limit our future selves. 🤔