Feature/david/contrast colors change #838
Conversation
# Conflicts: # app/src/main/res/values/attrs.xml # app/src/main/res/values/themes.xml
…trast_colors_change
…e/david/contrast_colors_change
…contrast_colors_change # Conflicts: # app/src/main/java/com/duckduckgo/app/bookmarks/ui/BookmarksActivity.kt
| val message = getString(R.string.bookmarkDeleteConfirmMessage, bookmark.title).html(this) | ||
| val title = getString(R.string.dialogConfirmTitle) | ||
| deleteDialog = AlertDialog.Builder(this) | ||
| deleteDialog = MaterialAlertDialogBuilder(this) |
There was a problem hiding this comment.
because we want to change the background color of the dialogs, we move to MaterialAlertDialog and apply a default theme
|
|
||
| } | ||
| private fun showOverFlowMenu(anchor: ImageView, bookmark: BookmarkEntity) { | ||
| val popupMenu = BookmarksPopupMenu(layoutInflater) |
There was a problem hiding this comment.
same here, need to change the PopupMenu colors
| val urlInput = rootView.find<EditText>(R.id.urlInput) | ||
|
|
||
| val alertBuilder = AlertDialog.Builder(requireActivity()) | ||
| val alertBuilder = MaterialAlertDialogBuilder(requireActivity()) |
|
|
||
| if (isShowing != true) { | ||
| alertDialog = AlertDialog.Builder(context) | ||
| alertDialog = MaterialAlertDialogBuilder(context) |
| <shape android:shape="rectangle"> | ||
| <padding android:top="1dp"/> | ||
| <solid android:color="#50CCCCCC" /> | ||
| <solid android:color="?attr/toolbarBgBorderColor" /> |
There was a problem hiding this comment.
these are preliminary changes, https://app.asana.com/0/1157893581871903/1176799629392268 will make sure to update these changes should the experiment move forward
marcosholgado
left a comment
marcosholgado
left a comment
There was a problem hiding this comment.
Code wise just some minor comments. I also created https://app.asana.com/0/inbox/1125189844075764/1177618788912081/1177628356924635 to list different UI issues, we can continue the conversation there to see which one of them are actually issues.
| class BookmarksPopupMenu(layoutInflater: LayoutInflater, view: View = inflate(layoutInflater, R.layout.popup_window_bookmarks_menu)) : | ||
| PopupWindow(view, WRAP_CONTENT, WRAP_CONTENT, true) { | ||
|
|
||
| // popupwindow gets stuck on the screen on API 22 (tested on 23) without a background |
There was a problem hiding this comment.
nit: we already have this comment below
| // color. Adding it however garbles the elevation so we cannot have elevation here. | ||
| setBackgroundDrawable(ColorDrawable(Color.WHITE)) | ||
| } else { | ||
| elevation = 6.toFloat() |
There was a problem hiding this comment.
should this be in a const like MARGIN making it a Float and save the .toFloat()?
| android:paddingBottom="17dp" | ||
| android:text="@string/fireClearAll" | ||
| android:textColor="@color/brickOrange" | ||
| android:textColor="?fireIcon" |
There was a problem hiding this comment.
Consider changing this to fireIconColor to be consistent with the rest of the attrs which always specify in the name what they are.
|
@marcosholgado I've removed the changes to the Dialogs and it's now good for re-review. Addressed the comments you had too. Thanks for the review! |
marcosholgado
left a comment
marcosholgado
left a comment
There was a problem hiding this comment.
Looks good! Nice job!
|
Thanks for the review @marcosholgado ! |
2 participants
Task/Issue URL: https://app.asana.com/0/715106103902962/874107794298116
Product Review URL: https://app.asana.com/0/1142021229838617/1176459710279645
Description:
In a handful of scenarios in our Android app we fail to meet WCAG (Web Content Accessibility Guidelines https://www.w3.org/WAI/standards-guidelines/wcag/) standards. This results in a problematic user experience for people with visual impairments and suboptimal experience for everyone else.
This PR fixes those issues, using new colors that meet the above mentioned guidelines
Steps to test this PR:
Internal references:
Software Engineering Expectations
Technical Design Template