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

GR-10830: Add the Clear style to ThumbprintButton #56

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

alexroussiereTT
Copy link
Member

@alexroussiereTT alexroussiereTT commented Jan 5, 2023

What

We currently don't have a way to display a "clear" button, meaning a button that doesn't have a background color, nor border.
Even though we do have some clear button currently within the Customer app, the way they are implemented is via the local ButtonWithDrawable class using a style that is within the Thumbprint library.
This PR is for adding more support to the ThumbprintButton itself, so we can keep transitioning from the Client app ButtonWithDrawable to the ThumbprintButton

How

  • Looked at the Thumbprint.Button.Clear style within button_styles.xml and use it to create a CLEAR ThumbprintButtonType
  • Adding a clear buttonType attribute so we can set it via XML

Testing

  • You can create a test layout.xml and add a ThumbprintButton with a app:buttonType="clear" and check the preview

Personally I tested it by publishing to my local maven repo and used it directly within the client app. Looked good.

Screenshot

Screenshot 2023-01-04 at 4 05 54 PM

@alexroussiereTT alexroussiereTT force-pushed the GR-10830_LinkThumbprintButton branch from 0b446ec to c49a7f1 Compare January 5, 2023 15:45
@dzirbel
Copy link
Contributor

dzirbel commented Jan 5, 2023

Hm, is this part of Thumbprint officially? I don't see it in https://thumbprint.design/components/button/usage/. In the past I think I've just used a clickable TextView for cases like this, which might be more semantically accurate.

@mallikapotter
Copy link
Collaborator

mallikapotter commented Jan 5, 2023 via email

@alexroussiereTT
Copy link
Member Author

alexroussiereTT commented Jan 5, 2023

From what I see currently, we are using this:

 <!-- TODO(PUNK-1160) This is not defined in Thumbprint -->
    <style name="Thumbprint.Button.Clear">
        <item name="android:background">?android:attr/selectableItemBackground</item>
        <item name="android:textColor">@color/button_clear_text_color_selector</item>
    </style>

which makes it sound like it just never got added. But it's used 42 times currently in the app.
The iOS code base seems to have what they call a Link button that is a bit similar I guess.

@alexroussiereTT alexroussiereTT requested a review from egoens January 5, 2023 21:58
@alexroussiereTT
Copy link
Member Author

alexroussiereTT commented Jan 5, 2023

I will just add that the benefits of using a Button class are:

  • We ensure the 48dp clickable area, which is per accessibility/guideline (A TextView by default is only 24dp)
  • When we use an icon, we are having the same space between the text and the icon than any other buttons in the app, out of the box (no need to manually add the padding, and do the search of what the value should be)
  • It uses the right colors for the enable/disable states out of the box

@lasya-tack
Copy link
Member

Let's check with Erik Goens to see which we want to do -> add it or use a clickable TextView.

On Thu, Jan 5, 2023 at 12:31 PM Dominic Zirbel @.> wrote: Hm, is this part of Thumbprint officially? I don't see it in https://thumbprint.design/components/button/usage/. In the past I think I've just used a clickable TextView for cases like this, which might be more semantically accurate. — Reply to this email directly, view it on GitHub <#56 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/ACV5ODOEVJ7Z3EZJZZA3XHTWQ4VTNANCNFSM6AAAAAATSCZ2XU . You are receiving this because you are subscribed to this thread.Message ID: @.>

Fwiw, iOS has had a "LINK" button style for a very long time.
https://thumbprint.design/components/button/ios/
We also have LINK as a CtaTheme on graphql that is used to style buttons from server. Imo, supporting LINK style on android button makes sense to easily support server driven button styling.

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.

4 participants