[Reader] Increase `Reader` comments like count disabled state alpha #20445

RenanLukas · 2024-03-11T20:08:34Z

Fixes #20443

Important

✅ I'm concerned that the opacity for disabled state is too close to the enabled state, but if the opacity is lower than that it might not seem that the the like count is disabled (cc @osullivanchris in case you have any ideas). Do not merge until we have confirmation that this works from a design perspective.

To Test:

Install Jetpack and sign in with a self-hosted site.
Open Reader.
Select "Discover" feed.
Open any post with comments.
🔍 Open the comments list screen: the number of likes label and icon (e.g. 2 likes) should be visible.

Before (like count with state disabled)

After (like count with state disabled)

Like count with state enabled (just for reference since there were no changes targeting this state)

Regression Notes

Potential unintended areas of impact
- None
What I did to test those areas of impact (or what existing automated tests I relied on)
- Manual testing
What automated tests I added (or what prevented me from doing so)

--

PR Submission Checklist:

I have completed the Regression Notes.
I have considered adding accessibility improvements for my changes.
I have considered if this change warrants user-facing release notes and have added them to RELEASE-NOTES.txt if necessary.

Testing Checklist (strike-out the not-applying and unnecessary ones):

wpmobilebot · 2024-03-11T20:26:16Z

📲 You can test the changes from this Pull Request in Jetpack by scanning the QR code below to install the corresponding build.

	App Name	Jetpack
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr20445-2168f10
	Commit	`2168f10`
	Direct Download	`jetpack-prototype-build-pr20445-2168f10.apk`

Note: Google Login is not supported on these builds.

wpmobilebot · 2024-03-11T20:31:21Z

📲 You can test the changes from this Pull Request in WordPress by scanning the QR code below to install the corresponding build.

	App Name	WordPress
	Flavor	Jalapeno
	Build Type	Debug
	Version	pr20445-2168f10
	Commit	`2168f10`
	Direct Download	`wordpress-prototype-build-pr20445-2168f10.apk`

Note: Google Login is not supported on these builds.

thomashorta

The changes look good to me, I just added a small suggestion to make things consistent with Material values.

Even though I kinda agree that disabled and medium emphasis look a bit similar, they are consistent with the system and Material values, and they do look different enough, in my opinion.

thomashorta · 2024-03-11T20:40:30Z

WordPress/src/main/res/color/on_surface_medium_secondary_selector.xml

+    <item android:alpha="@dimen/reader_comments_like_count_disabled_alpha" android:color="?attr/colorOnSurface" android:state_enabled="false" android:state_selected="false" />
+    <item android:alpha="@dimen/reader_comments_like_count_disabled_alpha" android:color="?attr/colorOnSurface" android:state_enabled="false" android:state_selected="true" />


💡 Suggestion: since we are already using the emphasis medium alpha from the material lib (line 8) why don't we also use the emphasis disabled alpha for this one, which is material_emphasis_disabled and has value 0.38.

That's the one used by default in material component texts and outlines so I think it would make sense to use that, and it's pretty close to what you're proposing here.

Good idea, thanks! ✅

…ng new value

sonarcloud · 2024-03-11T21:21:06Z

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

osullivanchris

in the future work I'm doing for comments, I'm separating the like count from the like action, as we do for posts. So this will be different then. Maybe not worth optimising the current one too much?

RenanLukas · 2024-03-12T20:10:13Z

Thanks for the review, @osullivanchris

Maybe not worth optimising the current one too much?

Makes sense 👍 (cc @develric since you reported the issue)

develric

Thanks for the ping @RenanLukas 🙇 . What Chris says makes sense 👍 . I just gave it a quick test after the changes requested from Thomas and it's looking good, also code LGTM as well. I think it's a nice interim improvement

Side note: comment improvements look really nice (the sample screen shared but also some neat early-stage brainstorming ideas on the topic I happened to see). Cannot wait for us to contribute to it ❤️

Increase Reader comments like count disabled state alpha

a62d139

RenanLukas added Reader Comments Reader IA labels Mar 11, 2024

RenanLukas requested review from daniloercoli, thomashorta, develric and osullivanchris March 11, 2024 20:08

RenanLukas marked this pull request as ready for review March 11, 2024 20:09

thomashorta reviewed Mar 11, 2024

View reviewed changes

Apply PR suggestion: use material_emphasis_disabled instead of creati…

2168f10

…ng new value

RenanLukas added the Do Not Merge In PRs with this label, our automation will fail a require check, preventing accidental merging label Mar 11, 2024

osullivanchris reviewed Mar 12, 2024

View reviewed changes

RenanLukas removed the Do Not Merge In PRs with this label, our automation will fail a require check, preventing accidental merging label Mar 13, 2024

develric approved these changes Mar 13, 2024

View reviewed changes

RenanLukas merged commit ba91e0f into trunk Mar 13, 2024
20 of 24 checks passed

RenanLukas deleted the issue/20443-comments-like-count-not-clearly-visible-with-ui-disabled branch March 13, 2024 16:07

RenanLukas added this to the 24.5 milestone Mar 13, 2024

RenanLukas self-assigned this Mar 15, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Reader] Increase `Reader` comments like count disabled state alpha #20445

[Reader] Increase `Reader` comments like count disabled state alpha #20445

RenanLukas commented Mar 11, 2024 •

edited

Loading

wpmobilebot commented Mar 11, 2024 •

edited

Loading

wpmobilebot commented Mar 11, 2024 •

edited

Loading

thomashorta left a comment

thomashorta Mar 11, 2024

RenanLukas Mar 11, 2024

sonarcloud bot commented Mar 11, 2024

osullivanchris left a comment

RenanLukas commented Mar 12, 2024 •

edited

Loading

develric left a comment •

edited

Loading

		<item android:alpha="@dimen/reader_comments_like_count_disabled_alpha" android:color="?attr/colorOnSurface" android:state_enabled="false" android:state_selected="false" />
		<item android:alpha="@dimen/reader_comments_like_count_disabled_alpha" android:color="?attr/colorOnSurface" android:state_enabled="false" android:state_selected="true" />

[Reader] Increase Reader comments like count disabled state alpha #20445

[Reader] Increase Reader comments like count disabled state alpha #20445

Conversation

RenanLukas commented Mar 11, 2024 • edited Loading

To Test:

Before (like count with state disabled)

After (like count with state disabled)

Like count with state enabled (just for reference since there were no changes targeting this state)

Regression Notes

PR Submission Checklist:

Testing Checklist (strike-out the not-applying and unnecessary ones):

wpmobilebot commented Mar 11, 2024 • edited Loading

wpmobilebot commented Mar 11, 2024 • edited Loading

thomashorta left a comment

Choose a reason for hiding this comment

thomashorta Mar 11, 2024

Choose a reason for hiding this comment

RenanLukas Mar 11, 2024

Choose a reason for hiding this comment

sonarcloud bot commented Mar 11, 2024

Quality Gate passed

osullivanchris left a comment

Choose a reason for hiding this comment

RenanLukas commented Mar 12, 2024 • edited Loading

develric left a comment • edited Loading

Choose a reason for hiding this comment

[Reader] Increase `Reader` comments like count disabled state alpha #20445

[Reader] Increase `Reader` comments like count disabled state alpha #20445

RenanLukas commented Mar 11, 2024 •

edited

Loading

wpmobilebot commented Mar 11, 2024 •

edited

Loading

wpmobilebot commented Mar 11, 2024 •

edited

Loading

RenanLukas commented Mar 12, 2024 •

edited

Loading

develric left a comment •

edited

Loading