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

Mobile Gallery Block - Known Issues #1674

Open
8 of 14 tasks
mkevins opened this issue Dec 12, 2019 · 5 comments
Open
8 of 14 tasks

Mobile Gallery Block - Known Issues #1674

mkevins opened this issue Dec 12, 2019 · 5 comments
Assignees
Labels
[Type] Enhancement Improves a current area of the editor

Comments

@mkevins
Copy link
Contributor

mkevins commented Dec 12, 2019

Known issues

This for tracking known issues for mobile gallery block


  • On the block settings sheet, “Images Size” row label should be “Image Size"

  • Android - “Link To” and “Image Size” bottom sheets shouldn’t have “Cancel” button

  • Android - Not specific to the Gallery block (see Video block), but the block settings bottom sheet feels a little tight at the bottom of the sheet. Can we add ~16px under the last list item?

  • Performance seems to suffer when there are ~15+ gallery images (only seeing this on Android)


  • Caption should have a subtle "shadow" gradient, going from 0% black (top) to 100% black (bottom), with a stop at around 70% from top. (specific design spec in the Zeplin project). Note: this is very similar to the way web styles it, but refined a bit.

This one may be tricky, as I don't believe gradients are supported out of the box in RN. There is a library https://github.com/react-native-community/react-native-linear-gradient, but I didn't see that we are using it. This might require some exploration.

I have added some simple transparent dark style as the caption background for now, but this can be improved in later iterations.

Edit:
react-native-linear-gradient may soon be included as a dependency and available within the project.


Note: iOS only, also, might be an issue with RangeControl

Tracked here: #1767


  • Long-pressing on a selected image should show the Replace Action/Bottom Sheet. Another thing regarding replacement: we might want to show the replace icon (see new icon in image block toolbar) when an image is selected. This isn't possible on web but imo it's an obvious thing.

  • Caption padding seems a bit off (should be 8px)
    • Android: the top is too small, the bottom too large
    • iOS: the top and bottom padding are both too small
Screenshot (Android)

IMG_7353


✔️ Fixed issues


  • Last cell in gallery settings bottom sheet shouldn’t have a separator
Screenshot

image

PR: #1766


  • “Link To” cell value (on right) should be title case and same label as Action/Bottom Sheet values
Screenshot

image

Tracked here: #1764


  • Appender icon ➕ is not the same color as the others (see inline toolbar icons).
Screenshot

image

PR: #1766


  • Generated excerpts show gallery captions without spacing between the captions making them hard to read

Tracked here: wordpress-mobile/WordPress-Android#11063
Fixed by: wordpress-mobile/WordPress-Android#11138

@mkevins mkevins self-assigned this Dec 12, 2019
@mchowning
Copy link
Contributor

mchowning commented Dec 13, 2019

👋 @mkevins ! I was doing some testing for the 1.19 release, and I noticed a weird issue with gallery block captions on Android. In particular, I was unable to apply formatting if the first word of the caption was highlighted, but I could apply formatting if anything other than the first word was highlighted. 😕 I did not observe this issue for the top-level captions for gallery/image/video blocks.

cc: @geriux who I believe is working on refactoring the captions right now in case he's observed anything similar to this.

gallery_caption_format_beginning_android mp4

Also, on iOS I observed one time where I was not able to apply any formatting to captions. I have not been able to reproduce this iOS issue, so it seems very likely that this was just a temporary local issue for me, but I wanted to mention it so it was on your radar in case you happened to see something similar.

Proof that I'm not crazy and that I really did see iOS refusing to format captions

ios-caption-style-failure mov

@mkevins
Copy link
Contributor Author

mkevins commented Dec 16, 2019

Thanks @mchowning for reporting 👍 . I was also able to reproduce this. Additionally, when I added a space in front of the first word (e.g. " Test") then selected that word and pressed bold, the "T" was not bold, but "est" was 🤔 . The resulting html looked like: <figcaption ...>T<strong>est </strong>.... This makes me suspicious whether we have some kind of "off-by-one" error for the RichText on the native side.

I did not observe this issue for the top-level captions for gallery/image/video blocks.

One difference we have for gallery captions on Android is the tagName prop. To avoid a bug preventing newlines, it is currently set to '' (empty string), instead of p. I'm not sure whether it's related, but might be worth trying (i.e. see if the bug goes away with tagName='p', and investigating further from there).

Also, on iOS I observed one time where I was not able to apply any formatting to captions. I have not been able to reproduce this iOS issue, so it seems very likely that this was just a temporary local issue for me, but I wanted to mention it so it was on your radar in case you happened to see something similar.

I wonder if any special handling on the native side for RichText could be resulting in these symptoms, based on some intermittent conditions. In any case, it may be a clue, so thanks for reporting that.

@geriux
Copy link
Contributor

geriux commented Dec 16, 2019

One difference we have for gallery captions on Android is the tagName prop. To avoid a bug preventing newlines, it is currently set to '' (empty string), instead of p. I'm not sure whether it's related, but might be worth trying (i.e. see if the bug goes away with tagName='p', and investigating further from there).

Just tried setting the tagName p as we do for iOS and it allows you to format the first word correctly, but we can't add new lines with it.

@mchowning
Copy link
Contributor

To avoid a bug preventing newlines, it is currently set to '' (empty string), instead of p.

FWIW, the reason that using p tags break the enter key behavior is because AztecEditor-Android removes <br> tags that are inside of<p> tags. I discussed this some in one of the original caption PRs.

@chipsnyder
Copy link
Contributor

As we chip away at these issues it might be worth while to take on #2737 as well

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Enhancement Improves a current area of the editor
Projects
None yet
Development

No branches or pull requests

5 participants