Updates to writing prompt block in full post reader #76092

davemart-in · 2023-04-21T15:43:22Z

Description

It was brought to my attention yesterday that the new writing prompts block was being shown on the reader full post view without any formatting. This in turn is affecting the native mobile apps. There is an internal discussion related to the mobile apps here: pctCYC-Mc-p2

Before

After

Testing Instructions

Apply this PR locally or click the "Calypso live" link below
Navigate to /read/blogs/167232230/posts/5558

github-actions · 2023-04-21T15:43:34Z

Calypso Live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-64776

Jetpack Cloud live (direct link)

	https://calypso.live?image=registry.a8c.com/calypso/app:build-64776&env=jetpack

matticbot · 2023-04-21T15:50:24Z

This PR does not affect the size of JS and CSS bundles shipped to the user's browser.

Generated by performance advisor bot at iscalypsofastyet.com.

creativecoder

This looks much better! I left a couple of suggestions, otherwise looks good to go.

client/blocks/reader-full-post/_content.scss

thomashorta

I added a comment in the original reference (pctCYC-Mc-p2#comment-847) with some issues I found on the mobile app reader and I also added some comments about them here, for the ones I could identify the cause.

client/blocks/reader-full-post/_content.scss

client/assets/images/icons/light-bulb-dark.svg

davemart-in · 2023-04-25T13:18:49Z

@thomashorta please have another look when you have a moment. I believe we should be close!

davemart-in · 2023-04-25T13:50:09Z

@thomashorta I saw a comment come through from you via email, but I don't see it in the thread above for some reason. I removed the dark color scheme preference code in 8de7391

thomashorta

Thanks for the changes, I think we are very close!!! I just added a comment about an issue regarding the usage of prefers-color-scheme that I just realized.

thomashorta · 2023-04-25T14:01:05Z

client/blocks/reader-full-post/_content.scss

+	}
+
+	@media (prefers-color-scheme: dark) {
+		.reader-full-post__story-content .jetpack-blogging-prompt .jetpack-blogging-prompt__label::before {


I believe this @media query needs to be nested inside .jetpack-blogging-prompt__label and this selector could be just &::before.

Currently, the compiled css is resulting in this matcher, because of the nesting:

.reader-full-post__story-content .jetpack-blogging-prompt .reader-full-post__story-content .jetpack-blogging-prompt .jetpack-blogging-prompt__label::before

Fixing the above makes things look good on mobile! 🎉

BUT doing so introduces a different problem that I just realized today. Since this CSS is also being used for web, this media query will also apply on web browsers, which is not desirable, since even on a Dark theme, the Reader web page has a white background. Here's an example, injecting the resulting CSS on Chrome and switching between MacOS' Themes:

reader-media-prefer-color-scheme-browser-icon.mov

thomashorta · 2023-04-25T14:13:44Z

I saw a comment come through from you via email, but I don't see it in the thread above for some reason. I removed the dark color scheme preference code in 8de7391

Sorry @davemart-in, GitHub is trolling me, I wrote that comment, and after I posted the page refreshed with your changes for today, so I deleted it in order to test it and make sure that was an actual problem, but it is.

I am not sure what would be the best way of achieving the correct icon color on mobile without making hardcoded workarounds in both Android and iOS WebView "renderers". 😞

Maybe we should inject a .darkmode class somewhere so we can have better control of these things from the css when needed, as you commented here. 🤔

davemart-in · 2023-04-25T15:28:13Z

I'm out of ideas as well. Happy to proceed however you think is best. We could:

A) Just ship what I have in this PR and let you deal with the dark mode issue in each of the native implementations.
B) Add a CSS class to distinguish a .darkmode, which would then need to be added to both native apps.

Which do you prefer @thomashorta?

thomashorta · 2023-04-25T19:13:41Z

I'm out of ideas as well. Happy to proceed however you think is best. We could:

A) Just ship what I have in this PR and let you deal with the dark mode issue in each of the native implementations.
B) Add a CSS class to distinguish a .darkmode, which would then need to be added to both native apps.

Which do you prefer @thomashorta?

I think we could ship with what you have in this PR for now, with the --color-neutral-10, since that is already supported by the apps so deploying this change would make the block work basically 100% correctly in all clients. I do have to make a few minor changes in the mobile code and also figure out the icon color for dark mode on mobile.

Btw, regarding that icon color on mobile, I was wondering if it would make sense to add the specific media query for dark mode in the reader-mobile.scss file, since rules there would (AFAIK) only affect the mobile apps.

If that's possible and makes sense, adding the lines below to that file would make the block work 99% correctly on all clients, without any mobile-side changes. That missing 1% is a minor bug I found on Android that needs to be fixed in the app.

@media (prefers-color-scheme: dark) {
    .reader-full-post__story-content .jetpack-blogging-prompt .jetpack-blogging-prompt__label::before {
        // stylelint-disable-next-line function-url-quotes
        background-image: url("data:image/svg+xml,%3Csvg fill='none' height='20' viewBox='0 0 20 20' width='20' xmlns='http://www.w3.org/2000/svg'%3E%3Cpath d='m12.3438 17.3438h-4.68755c-.08594 0-.15625.0703-.15625.1562v.625c0 .3457.2793.625.625.625h3.75c.3457 0 .625-.2793.625-.625v-.625c0-.0859-.0703-.1562-.1562-.1562zm-2.3438-16.0938c-3.53711 0-6.40625 2.86914-6.40625 6.40625 0 2.37105 1.28906 4.44145 3.20313 5.54885v2.2637c0 .3457.27929.625.625.625h5.15622c.3457 0 .625-.2793.625-.625v-2.2637c1.9141-1.1074 3.2032-3.1778 3.2032-5.54885 0-3.53711-2.8692-6.40625-6.4063-6.40625zm2.498 10.7383-.7011.4062v2.293h-3.59377v-2.293l-.70118-.4062c-1.53711-.8887-2.50195-2.52541-2.50195-4.33205 0-2.76172 2.23828-5 5-5 2.7617 0 5 2.23828 5 5 0 1.80664-.9648 3.44335-2.502 4.33205z' fill='%23fff'/%3E%3C/svg%3E");
    }
}

What do you think?

davemart-in · 2023-04-25T20:08:44Z

@thomashorta, yep! That is correct. Updated in 1806f5c

thomashorta

That is correct. Updated in 1806f5c

Awesome!!! Now it seems to be working correctly on Android, iOS, and Web! 🎉

Thanks a lot for the changes and sorry again for the back and forth! 🙇

Android	iOS

…ence

davemart-in · 2023-04-25T20:29:56Z

There were two seemingly unrelated E2E test failures, so I just rebased the branch to see if that would dismiss them.

davemart-in added [Type] Bug [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. [Feature] Reader The reader site on Calypso. [Size] S Small sized issue writing prompts Writing prompts project labels Apr 21, 2023

davemart-in requested a review from creativecoder April 21, 2023 15:43

davemart-in self-assigned this Apr 21, 2023

davemart-in requested a review from a team as a code owner April 21, 2023 15:43

creativecoder reviewed Apr 21, 2023

View reviewed changes

client/blocks/reader-full-post/_content.scss Outdated Show resolved Hide resolved

client/blocks/reader-full-post/_content.scss Show resolved Hide resolved

thomashorta reviewed Apr 21, 2023

View reviewed changes

client/blocks/reader-full-post/_content.scss Show resolved Hide resolved

client/blocks/reader-full-post/_content.scss Outdated Show resolved Hide resolved

client/blocks/reader-full-post/_content.scss Outdated Show resolved Hide resolved

creativecoder reviewed Apr 24, 2023

View reviewed changes

client/assets/images/icons/light-bulb-dark.svg Outdated Show resolved Hide resolved

thomashorta reviewed Apr 25, 2023

View reviewed changes

thomashorta approved these changes Apr 25, 2023

View reviewed changes

davemart-in added 9 commits April 25, 2023 16:28

Updates to writing prompt block in full post reader

ae881e7

Some minor updates to see if we can get this working on native mobile.

cda4187

Brought icon into Calypso

d8dc83f

Added blockquote style

9230418

Updates to help native mobile apps

b4d2a26

Update to line color for native mobile apps

2566467

Dark mode icon variation

82d6242

Change border color back to a variable

95924a1

Moved background icon image inline and added dark color scheme prefer…

a814cee

…ence

davemart-in added 3 commits April 25, 2023 16:28

Removed dark color scheme preference

0399ae7

Changed to --color-neutral-10 to help native apps

9271c3e

Adding native mobile styles for dark mode

0c922a3

davemart-in force-pushed the fix/reader-prompt-block-formatting branch from 1806f5c to 0c922a3 Compare April 25, 2023 20:29

This was referenced Apr 25, 2023

[Reader] Add support to Prompt block wordpress-mobile/WordPress-Android#18323

Merged

[Reader] Add --color-neutral-5 to ReaderWebView CSS colors wordpress-mobile/WordPress-iOS#20608

Merged

davemart-in merged commit 808d0f1 into trunk Apr 26, 2023

davemart-in deleted the fix/reader-prompt-block-formatting branch April 26, 2023 11:55

github-actions bot removed the [Status] Needs Review The PR is ready for review. This also triggers e2e canary tests and wp-desktop tests automatically. label Apr 26, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Updates to writing prompt block in full post reader #76092

Updates to writing prompt block in full post reader #76092

davemart-in commented Apr 21, 2023

github-actions bot commented Apr 21, 2023 •

edited by matticbot

Loading

matticbot commented Apr 21, 2023

creativecoder left a comment

thomashorta left a comment •

edited

Loading

davemart-in commented Apr 25, 2023

davemart-in commented Apr 25, 2023

thomashorta left a comment

thomashorta Apr 25, 2023

thomashorta commented Apr 25, 2023 •

edited

Loading

davemart-in commented Apr 25, 2023

thomashorta commented Apr 25, 2023 •

edited

Loading

davemart-in commented Apr 25, 2023

thomashorta left a comment

davemart-in commented Apr 25, 2023

Updates to writing prompt block in full post reader #76092

Updates to writing prompt block in full post reader #76092

Conversation

davemart-in commented Apr 21, 2023

Description

Before

After

Testing Instructions

github-actions bot commented Apr 21, 2023 • edited by matticbot Loading

matticbot commented Apr 21, 2023

creativecoder left a comment

Choose a reason for hiding this comment

thomashorta left a comment • edited Loading

Choose a reason for hiding this comment

davemart-in commented Apr 25, 2023

davemart-in commented Apr 25, 2023

thomashorta left a comment

Choose a reason for hiding this comment

thomashorta Apr 25, 2023

Choose a reason for hiding this comment

thomashorta commented Apr 25, 2023 • edited Loading

davemart-in commented Apr 25, 2023

thomashorta commented Apr 25, 2023 • edited Loading

davemart-in commented Apr 25, 2023

thomashorta left a comment

Choose a reason for hiding this comment

davemart-in commented Apr 25, 2023

github-actions bot commented Apr 21, 2023 •

edited by matticbot

Loading

thomashorta left a comment •

edited

Loading

thomashorta commented Apr 25, 2023 •

edited

Loading

thomashorta commented Apr 25, 2023 •

edited

Loading