Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

Fixes #7041: Adds an attribute to change login prompt editText cursor color #7056

Merged
merged 2 commits into from
May 22, 2020
Merged

Fixes #7041: Adds an attribute to change login prompt editText cursor color #7056

merged 2 commits into from
May 22, 2020

Conversation

ValentinTimisica
Copy link
Contributor

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks.
  • Tests: This PR does not include thorough tests. Small UI change.
  • Changelog: This PR includes a changelog entry.
  • Accessibility: The code in this PR follows accessibility best practices.

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

@codecov
Copy link

codecov bot commented May 21, 2020 •
edited
Loading

Codecov Report

Merging #7056 into master will decrease coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #7056      +/-   ##
============================================
- Coverage     77.17%   77.04%   -0.14%     
- Complexity     4882     4885       +3     
============================================
  Files           645      651       +6     
  Lines         24053    24096      +43     
  Branches       3535     3536       +1     
============================================
+ Hits          18563    18564       +1     
- Misses         4006     4049      +43     
+ Partials       1484     1483       -1
Impacted Files Coverage Δ Complexity Δ
...ozilla/components/support/android/test/LiveData.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...port/android/test/espresso/matcher/ViewMatchers.kt 100.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...ozilla/components/support/android/test/Matchers.kt 100.00% <0.00%> (ø) 3.00% <0.00%> (?%)
...onents/support/android/test/rules/WebserverRule.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...s/support/android/test/espresso/ViewInteraction.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...ts/support/android/test/leaks/LeakDetectionRule.kt 0.00% <0.00%> (ø) 0.00% <0.00%> (?%)
...components/support/sync/telemetry/SyncTelemetry.kt 87.71% <0.00%> (+2.00%) 41.00% <0.00%> (ø%)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5d82a88...bbf0cb5. Read the comment docs.

Amejia481
Amejia481 reviewed May 21, 2020
View reviewed changes
components/feature/prompts/src/main/res/layout/mozac_feature_prompt_login_prompt.xml
@@ -86,7 +86,6 @@

<com.google.android.material.textfield.TextInputEditText
android:id="@+id/password_field"
style="@style/Widget.MaterialComponents.TextInputLayout.OutlinedBox"
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we use MozacPromptLoginTextInputLayoutCursorAppearance style here? I don't see how they linked :)

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh I see it gets its value from the parent that has the style MozTextInputLayout, right?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Right, from the parent.

@Amejia481 Amejia481 self-assigned this May 22, 2020
Amejia481
Amejia481 reviewed May 22, 2020
View reviewed changes
components/feature/prompts/src/main/res/values/styles.xml
<style name="MozacPromptLoginTextInputLayoutCursorAppearance" parent="ThemeOverlay.MaterialComponents.TextInputEditText.OutlinedBox">
<item name="colorControlActivated">?mozacPromptLoginEditTextCursorColor</item>
Copy link
Contributor

Choose a reason for hiding this comment

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

Where are we proving the value for ?mozacPromptLoginEditTextCursorColor? As we don't have a default value what will happen for consumer that doesn't provide this value?

Copy link
Contributor

Choose a reason for hiding this comment

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

Do you have a sample code where we are providing the value

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I use the attribute in Fenix in styles.xml like this:
<item name="mozacPromptLoginEditTextCursorColor">@color/prompt_login_edit_text_cursor_color_normal_theme</item>
The same for private mode.

Regarding the default value, I tested on Fenix and if I don't use at all
?mozacPromptLoginEditTextCursorColor it works as before (the cursor color is the same as before - the color from the Fenix issue). But I don't know what will happen for other apps.

Any suggestions? I'm open to any change.

Copy link
Contributor

Choose a reason for hiding this comment

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

No problem, I just wanted to confirm that we don't have a crash when we don't provide a value. 👍

@Amejia481
Copy link
Contributor

bors r+

@Amejia481 Amejia481 added the 🛬 needs landing PRs that are ready to land label May 22, 2020
@bors
Copy link

bors bot commented May 22, 2020

Build succeeded:

@bors bors bot merged commit 9be6690 into mozilla-mobile:master May 22, 2020
ValentinTimisica added a commit to ValentinTimisica/fenix that referenced this pull request May 29, 2020
@ValentinTimisica
Fixes mozilla-mobile#10113: Changes login prompt editText cursor color
2cbb7ed 
This patch uses `mozacPromptLoginEditTextCursorColor` attribute defined in AC.
Check PR: mozilla-mobile/android-components#7056
@ValentinTimisica ValentinTimisica mentioned this pull request May 29, 2020
Merged
4 tasks
ekager pushed a commit to mozilla-mobile/fenix that referenced this pull request Jun 2, 2020
@ValentinTimisica @ekager
Fixes #10113: Changes login prompt editText cursor color
1c36708 
This patch uses `mozacPromptLoginEditTextCursorColor` attribute defined in AC.
Check PR: mozilla-mobile/android-components#7056
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@Amejia481 Amejia481 Amejia481 approved these changes

Assignees

@Amejia481 Amejia481

Labels
🛬 needs landing PRs that are ready to land
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@ValentinTimisica @Amejia481