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

[⚠️] NT-1131 Pledge error Toasts #832

Merged
merged 2 commits into from
Apr 10, 2020
Merged

[⚠️] NT-1131 Pledge error Toasts #832

merged 2 commits into from
Apr 10, 2020

Conversation

eoji
Copy link
Contributor

@eoji eoji commented Apr 9, 2020

📲 What

Beautiful new error Toast in the pledge screen

🤔 Why

The footer hid the old Snackbar

🛠 How

ToastExt

  • 🆕 shiny helper function to show error Toast

PledgeFragment

  • Using shiny 🆕 aforementioned helper function to show pledge errors

XML

  • Added ic_icon_alert.xml
  • Created a new layout for the error Toasts toast_error.xml
  • Added Alert a11y string

👀 See

Still GIF a11y
device-2020-04-09-153605 2020-04-09 16_24_46

📋 QA

IDK what kind of shenanigans TalkBack is on but it refuses to read the Alert icon first ಠ_ಠ

Story 📖

NT-1131

- 🆕 shiny helper function to show error `Toast`s
## `PledgeFragment`
- Using shiny 🆕 aforementioned helper function to show pledge errors
## XML
- Added `ic_icon_alert.xml`
- Created a new layout for the error `Toast`s `toast_error.xml`
- Added `Alert` a11y string
Copy link
Contributor

@Scollaco Scollaco left a comment

Choose a reason for hiding this comment

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

Looks good, @eoji. Just have a minor naming suggestion, but not a blocker.


fun showErrorToast(applicationContext: Context, root: ViewGroup, message: String) {
val layout: ViewGroup = LayoutInflater.from(applicationContext).inflate(R.layout.toast_error, root, false) as ViewGroup
val text: TextView = layout.findViewById(R.id.toast_text_view)
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe textView would be a better name, instead of simply text.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

android:id="@+id/toast_text_view"
style="@style/Subheadline"
android:layout_width="0dp"
android:layout_height="wrap_content"
Copy link
Contributor

Choose a reason for hiding this comment

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

I tried changing the traversal order by setting android:accessibilityTraversalAfter="@id/toast_icon here, but it didn't work either 🤷🏻‍♂️ Maybe this value is being overridden somewhere?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, like the dang Toast is doing something weird. I know that traversal is based on the top most view but even setting the size of the icon to the size of the container didn't work 😒

@eoji eoji merged commit 5f97849 into NT-1103 Apr 10, 2020
@eoji eoji deleted the NT-1131 branch April 10, 2020 22:28
eoji added a commit that referenced this pull request Apr 20, 2020
* [🦶🏾] NT-1104 Add Pledge screen footer (#817)
* [⬇] NT-1120 Vertical payment methods in Pledge screen (#821)
* [🔨] NT-1121 Stored cards redesign (#822)
* [🛒] NT-1105 Kickstarter is not a store redesign (#823)
* [💳] NT-1122 Initial card selection (#824)
* [⏳] NT-1124 Loading state for payment methods (#825)
* [➕] NT-1125 New card CTA (#828)
* [💳] NT-1123 Last four digits copy (#826)
* [💳] NT-1123 Card not allowed copy (#827)
* [🎨] NT-1111 Adding 2 buttons in the new pledge footer (#830)
* [🔌] NT-1129 Adding progress state for pledging (#831)
* [🗣] NT-1126 Payment methods a11y (#829)
* [👷🏾‍♀️] NT-1128 Moved continue button in Pledge screen to footer (#833)
* [⚠️] NT-1131 Pledge error Toasts (#832)
* [␡] NT-1127 Removing Update pledge button (#834)
* [📝] NT-1130 Pledge button CTA (#835)
* [✅] NT-1133 Enabling pledge button only when all fields are valid (#836)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants