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

Port Verse block to mobile #1886

Closed
3 of 6 tasks
guarani opened this issue Feb 12, 2020 · 28 comments
Closed
3 of 6 tasks

Port Verse block to mobile #1886

guarani opened this issue Feb 12, 2020 · 28 comments
Assignees
Labels

Comments

@guarani
Copy link
Contributor

guarani commented Feb 12, 2020

Description

The aim of this task is to implement the verse block on the mobile Gutenberg editor. The verse block is characterized by its support for line-breaks and ability to render arbitrary whitespace.

Considerations

The features to implement on mobile may be a subset of those found in Gutenberg web.

Subtasks

The subtasks that make up the verse block port are divided into those that are blocking, non-blocking, and undecided (still under investigation).

Blocking

Non-blocking

Additional Information

Support verse blocks in the Reader wordpress-mobile/WordPress-Android#11299, wordpress-mobile/WordPress-iOS#13452 (these are not Gutenberg per se, just tracking their progress here)

@guarani guarani closed this as completed Feb 12, 2020
@guarani guarani reopened this Feb 12, 2020
@guarani
Copy link
Contributor Author

guarani commented Feb 12, 2020

Verse Block vs. Paragraph Block on Gutenberg Web

When porting verse block features from web to mobile, this feature list may be useful to a) enumerate verse block features on web, b) compare the verse and paragraph blocks on the web to help leverage existing code at implementation time, and c) open discussion about which features of the verse block will be ported to mobile and which will be left out.

Feature Web Verse Support Web Paragraph Support Comments
Soft line-break Hit Return in the verse block, or Shift+Return in the paragraph block
Hard line-break 🛑 Not supported by verse block, hit Return in the paragraph block
Arbitrary whitespace 🛑 The paragraph block strips extra whitespace
Bold/Italic/Strikethrough -
Align text -
Uppercase text -
Inline code -
Additional CSS classes -
Hyperlinks -
Inline images -
Transform to paragraph block not applicable -
Transform to group block -

Other Considerations

Use cases for the verse block include quoting poetry and song lyrics. Pasting content through the device clipboard should be considered a likely scenario.

@guarani guarani added Blocks [Status] Needs Design Review Needs design review or sign-off before shipping labels Feb 12, 2020
@guarani guarani self-assigned this Feb 12, 2020
@iamthomasbishop
Copy link
Contributor

@guarani Thanks for jumping on this block! Stoked to see we're on it 😄

I took a look at how the web handles design/styling, and it largely depends on the active theme style.

Using Twenty Twenty theme

Styled similarly to Preformatted block.

image

Using Maywood theme

Styled like a Paragraph, but with additional padding.

image

Using Twenty Nineteen

Styled like a paragraph, no additional padding.

image

Proposal

In terms of styling, I propose that we should follow the Twenty Nineteen model. In other words:

  • No additional padding
  • Text styled like a paragraph
  • No borders
  • No background

@guarani
Copy link
Contributor Author

guarani commented Feb 12, 2020

Thanks for looking at the design, @iamthomasbishop!

On a slightly different topic, I noticed that when shown in the Reader, the verse block is just shown as pre-formatted text. My feeling is that this will be handled in a separate issue – and in the WordPress for iOS/Android repo – but would like to hear others' opinions.

Web Browser Reader (Android) Reader (iOS)

@iamthomasbishop
Copy link
Contributor

That’s a good reminder regarding Reader, @guarani! We should sync at some point with the other team that has Reader things in the apps in their view (perhaps @frosty can advise 🙂).

In terms of Reader styling, I’d suggest the Verse should look the same as I suggested for the editor (similar to paragraph — same font, size, no bg/padding, etc.), whereas code and pre should look similar to each other.

@frosty
Copy link

frosty commented Feb 13, 2020

Thanks for the ping! I think this definitely belongs in the iOS / Android repos – @guarani please could you open some issues for this?

@guarani
Copy link
Contributor Author

guarani commented Feb 14, 2020

Adding verse to the list of blocks to register in index.native.js was all that was necessary to see it on mobile.


Verse Block Demo

Here are some screenshots of the verse block running on iOS and Android.

Block Selector Verse Block
iOS
Android

Current State of Verse Block Support on Mobile

Here are the results of testing each of the features of the verse block on mobile.

Feature iOS Android Comments
Soft line-break ⚠️ Hitting return creates a new line. On iOS, new lines in pasted text are preserved. In Android, new lines in pasted text are not preserved. Leading new lines and trailing new lines are preserved.
Hard line-break A hard line-break – or new paragraph – is achieved by creating a new verse.
Arbitrary whitespace Leading, trailing and mid-sentence whitespace is preserved.
Bold/Italic/Strikethrough/Hyperlinks 🛑 iOS only: Leading whitespace is stripped from lines when bold, italics, links, or strikethrough are applied.

Align text 🛑 🛑 This option is missing from the toolbar when a verse block is chosen.

Next Steps

Can UI for following features – while implemented on the web – be left out of scope on mobile? These are things that seem to be not generally supported on mobile at the moment.

  • Uppercase text
  • Inline code
  • Additional CSS classes
  • Inline images
  • Transform to paragraph block
  • Transform to group block

I also propose:

  • Adding the ability for the user to align text, as it is already implemented for paragraph blocks
  • Preserving new lines when pasting text on Android (as noted above)
  • Update the styling to match what @iamthomasbishop mentioned above Port Verse block to mobile #1886 (comment)
  • Fixing the stripped whitespace bug (when bold, italics, links, or strikethrough are applied) on iOS

Regarding hyperlinks, I have not yet been able to test them in the Gutenberg demo app (through the simulators) because the link options (URL, Link text, new tab option) are not appearing, neither for the verse block nor the paragraph block. I'll have to dig further by testing locally through the WordPress native apps.

@hypest, @iamthomasbishop – does this all sound ok to you? I could start with text alignment and move from there.

@iamthomasbishop
Copy link
Contributor

Can UI for following features – while implemented on the web – be left out of scope on mobile?

I think they can be left out for this iteration, yes.

Thanks for the update!

@iamthomasbishop
Copy link
Contributor

Coming back to this issue, I was looking at the example above that shows indented lines, and it made me wonder if we should show indent and outdent buttons in the block toolbar like we do on the List block? This would essentially act as the sub for the tab key. Obviously this beyond scope for the current block, but it could be a useful addition. For visual example:

Note: We could also do this for the Code block as well (esp. if we want to weigh in on the tabs vs. spaces debate 😜).

@guarani
Copy link
Contributor Author

guarani commented Feb 19, 2020

Coming back to this issue, I was looking at the example above that shows indented lines, and it made me wonder if we should show indent and outdent buttons in the block toolbar like we do on the List block? This would essentially act as the sub for the tab key. Obviously this beyond scope for the current block, but it could be a useful addition. For visual example:

That's interesting, it hadn't occurred to me. If the indent/outdent button worked not only on single lines, but also on multi-line text selections, I think that would be really useful.

I agree this that indenting is a really useful feature for the code block.

@mchowning
Copy link
Contributor

mchowning commented Feb 20, 2020

I noticed some weird behavior when adding new lines and quickly typing that I think we should follow-up on before removing the dev flag on the block.

On Android, I see disappearing letters/words. iOS has that as well but also has seems to sometimes move characters and even have some new characters show up unexpectedly. I found it a bit tricky to reproduce this on WPiOS and Android debug builds (I didn't check a release build). On the Android demo app, however, the issue is relatively easy to reproduce.

_ WPiOS WPAndroid Android demo
Description disappearing characters, also can see new characters appearing disappearing characters disappearing characters/words
Difficulty to Reproduce Hardest Hard Moderate
gifs verse_weirdness-wpandroid mp4 verse_weirdness-android-demo mp4

UPDATE: I do not think this issue is tied to the verse block specifically, I also see similar behavior on the preformatted and list blocks. In light of that, I've opened a new issue to track this.

@guarani
Copy link
Contributor Author

guarani commented Feb 27, 2020

From #1886 (comment)

Fixing the stripped whitespace bug (when bold, italics, links, or strikethrough are applied) on iOS

I've broken this out into a separate bug issue with more context as it affects all blocks, not just the verse block. See #1966

@guarani
Copy link
Contributor Author

guarani commented Mar 25, 2020

Regarding styling (#1886 (comment)):

In terms of styling, I propose that we should follow the Twenty Nineteen model. In other words:

No additional padding
Text styled like a paragraph
No borders
No background

I want to use the exact same styles as the paragraph block, but from what I'm seeing, the paragraph block's styling on mobile is by virtue of it using the <p> tag and how Aztec interprets that – its styling doesn't come from scss as far as I can tell.

As a test, I changed the verse block to use a <p> tag instead of a <pre> tag and that achieved the styling I was looking for. This doesn't look like a solution though, because it would make the mobile editor output HTML for verse blocks which are fundamentally different from the web.

I could just try to play with the scss properties to make it visually match the paragraph block, or take a deep dive into Aztec to see what styling it's putting on <p> tags, but I'm not sure if I'm on the right track. Any thoughts here @hypest and @mchowning ?

@hypest
Copy link
Contributor

hypest commented Mar 26, 2020

A quick note:

On the mobile ports of the blocks, we never actually change the output markup of the block. In other words, we never need to change the save() function of the block. So, if there's a recommendation here to differentiate the frontend rendering of Verse block, I'd pause to rethink its need. I see that the current save() function of the Verse is using a pre

@guarani
Copy link
Contributor Author

guarani commented Mar 27, 2020

A recent change on Gutenberg (WordPress/gutenberg#20752) has fixed verse block styling – it's now the same as the paragraph block. Just noticed this today.

You can see in the below screenshots how the verse and paragraph blocks are now styled identically. I'm unclear on how this change works because under the hood the verse is still using the pre block (I switched to HTML mode to check this).

Android iOS

@hypest
Copy link
Contributor

hypest commented Mar 30, 2020

Aha, that's good news 😄 .

Ofc, having the PRE block look exactly like the paragraph is not desirable since the PRE should have a monospace font. I've opened a ticket for it #2081.

@guarani
Copy link
Contributor Author

guarani commented Mar 30, 2020

@iamthomasbishop, after discussion with @hypest and @SergioEstevao, the "fix" i mentioned above in #1886 (comment) is no longer the case. The verse block uses a pre tag under the hood to preserve spaces and line breaks, but Aztec always styles pre tags using a monospace font. Also, we can't change the Verse block to use a p tag as this would mean spaces and line-breaks are no longer preserved.

The proposal we considered is to leave the style of the verse block alone for now and address it at a later stage. What do you think @iamthomasbishop ?

@guarani
Copy link
Contributor Author

guarani commented Mar 30, 2020

Update to the above comment: we're also proposing leaving the "Bolding text using the toolbar causes whitespace to be lost" subtask out for now, so we can unblock the release of the Verse block which is still only available on development/debug builds.
It's already got its own issue: #1966

@iamthomasbishop
Copy link
Contributor

@guarani Apologies on the delay. I hope I'm understanding the issue properly. Is there anything we can latch onto that allows us to style the verse variation of pre in order to give it a monospace font?

we're also proposing leaving the "Bolding text using the toolbar causes whitespace to be lost" subtask out for now, so we can unblock the release of the Verse block

That sounds like a bug that we should fix prior to shipping the block, no?

@hypest
Copy link
Contributor

hypest commented Mar 31, 2020

Echoing Thomas above, the whitespace removal seems like a blocker for un-DEVing the block. What's the context behind the thought to not block on that issue?

@guarani
Copy link
Contributor Author

guarani commented Mar 31, 2020

Is there anything we can latch onto that allows us to style the verse variation of pre in order to give it a monospace font?

It's my understanding that we want to give it a non-monospace font, as per your comment here: #1886 (comment). Please let me know if there's been a misunderstanding.
If our goal is to give it a monospace font, I think that's already done – you can see what it currently looks like here, #1886 (comment).

That sounds like a bug that we should fix prior to shipping the block, no?

Echoing Thomas above, the whitespace removal seems like a blocker for un-DEVing the block. What's the context behind the thought to not block on that issue?

My reasoning is that since this is an existing bug in production (I'm not sure if I made that clear earlier), it's not specific to the verse block and equally affects any other rich text (e.g. pre-formatted and paragraph blocks). This doesn't mean it's not a good time to fix this bug now. Perhaps users of the verse block use styled and indented (leading whitespace) content more often than users of the pre-formatted or paragraph block.

@iamthomasbishop
Copy link
Contributor

It's my understanding that we want to give it a non-monospace font, as per your comment here

Ack yes, you’re right, I had it reversed 🤦‍♂

My reasoning is that since this is an existing bug in production (I'm not sure if I made that clear earlier), it's not specific to the verse block and equally affects any other rich text

Ok, that makes sense. I’d like to fix asap and pause shipping unless this fix will take a long time — in which case we can tackle separately.

Perhaps users of the verse block use styled and indented (leading whitespace) content more often than users of the pre-formatted or paragraph block.

I’m not sure — purely an assumption, but I’d assume indentation would probably also be common on the pre block, but not as common on the paragraph block.

@guarani
Copy link
Contributor Author

guarani commented Mar 31, 2020

No worries – I'm doing a bit more debugging on this and will write up my findings on the subissue in question, #1966. I'll ping you here once that's done 👍.

@guarani
Copy link
Contributor Author

guarani commented Apr 1, 2020

@iamthomasbishop, sorry to go back to what we were discussing earlier.
Could you please confirm if it is acceptable for the verse block to be released using a monospace font in the editor? This won't affect how the block is rendered on the web, it only affects how users see the verse block in the mobile editor.
I understand we would prefer that the verse block use the same font as the paragraph block in the editor, but in offline discussion with @hypest we concluded that would not be an easy fix.

@iamthomasbishop
Copy link
Contributor

Could you please confirm if it is acceptable for the verse block to be released using a monospace font in the editor?

I think it's acceptable, but hoping we can iterate on it quickly and apply the expected serif font.

@kspilarski
Copy link

Coming back to this issue, I was looking at the example above that shows indented lines, and it made me wonder if we should show indent and outdent buttons in the block toolbar like we do on the List block? This would essentially act as the sub for the tab key. Obviously this beyond scope for the current block, but it could be a useful addition. For visual example:

Note: We could also do this for the Code block as well (esp. if we want to weigh in on the tabs vs. spaces debate 😜).

Related GitHub report: Code & SyntaxHighlighting Blocks: Indent / Outdent Option

@mchowning
Copy link
Contributor

👋 @guarani ! Should we close this issue (and maybe creating new issues for things like indentation) or do you think we would be better off leaving this issue open?

@guarani
Copy link
Contributor Author

guarani commented Mar 19, 2021

This was mostly for tracking the effort needed to ship the first block, so I agree we could close this and add issues for the pending items. I'll do this soon, thanks for flagging this 👍

@guarani
Copy link
Contributor Author

guarani commented Apr 14, 2021

I created a separate issue to track the work that needs to be done to style the verse block like a paragraph block: WordPress/gutenberg#30836

The other non-blockers here all have their own issues already so I think this is safe to close.

@guarani guarani closed this as completed Apr 14, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants