[RNMobile] Buttons block #20191
[RNMobile] Buttons block #20191
Conversation
lukewalczak
added
[Status] In Progress
|
Hopefully, I covered all features required in the
|
|
@lukewalczak The spacing does look a little large, but I think it's something that could be improved in the next iteration if necessary. In general, I'm hesitant to recommend applying a special rule to a single block's inner blocks, however, I think we might see a similar issue on other blocks — such as the Navigation block or Social Buttons block — where there is a horizontal list of blocks that we want to be close to each other. I wonder if we should try to remove the horizontal spacing in-between "columns" (A) and rows (B). In other words, the dashed borders would be flush against each other. So the first button ( I'm not sure what would be the best way to do that, but I think we might want to build it in a way that allows us to use the same pattern for blocks like Nav, Social Buttons, etc. Let me know idf that all makes sense. |
|
Size Change: -8 B (0%) Total Size: 839 kB
ℹ️ View Unchanged
|
|
@iamthomasbishop I generated for you build which you can find there. Please give me feedback! Please notice that finally, I didn't change any spacings around buttons, because I've discovered that it's not so simple to adjust it for current styled and there is no universal styling also for other _horizontal_blocks.
If you would like I can add these styling changes and present them in another build as well, just let me know! |
|
Hey @lukewalczak, this is looking really solid! I have some picky feedback, but it already feels pretty workable and close to ready. Here are my thoughts:
Separate feedback that I wanted to note:
|
|
Thanks for valuable as always feedback! However, I would like to enquire more specifically about:
Does it mean that in the case present on the screenshot below, appender should appear on close to the selected button right side and a first button will go up if there is no space for it?
We are not presenting an appender at the end of the list, because instead, we have an inline appender, right? By pressing the trailing space you meant this marked area:
I think that we've discussed that already and the final decision was to open a block picker since automatically adding button block means architectural changes. Correct me if I understood it wrongly. |
|
According to pinned test cases I have tested on iOS and Android demo app TEST CASES
I will also proceed with sanity-test on production builds: AndroidButton-1
Button-2
Button-3
Button-4
edited |
|
@lukewalczak Giving this one more review and it feels really solid. The only big issues I have with it will be resolved by the progress being made on the floatingToolbar and color picker work. Note: I haven't had a chance to test Android yet because it's behind a flag — I will test again when I am able next week. I have one tiny design request — can we remove the border on the inline appender just for this block? I think we can keep the border on other instances of the inline appender (Columns, Gallery, Group, etc) because it looks okay on the full-width appender in those cases, but I think a more subtle button on this type of direct-inline adding makes more sense. It might be worth saving this "borderless" style as a variation of the appender because I have a feeling we're going to need it in the future (Navigation Menu block, Social Links, etc.). It would make the appender look like this:
Note: the tap area would still be the same size, this would just remove or make the border transparent. I apologize for not mentioning this earlier, but I didn't know if I was convinced at first, but now that I've had some time with the block and the dust has settled, I figured I should suggest it before we ship it. 😄 |
|
UPDATES Rename
|
|
@lukewalczak This is looking really solid, thanks for addressing the issues I mentioned previously — the floating appender looks much better. Layout-wise everything is feeling pretty good (other than the previously-mentioned floatingToolbar issue which is being resolved elsewhere). I noticed something with custom colors while testing side-by-side with the web app, but I'm not sure if this is because of some theme-color restrictions, or because of a regression. As far as I know, we were at one point supporting custom chosen colors for background and text, but I noticed that when I've applied custom background and text colors on the web, we're not inheriting those styles on the mobile editor. Here's an example:
|
|
Thanks @iamthomasbishop for reporting that. Some time ago was breakage related to colors so will investigate it! |
|
Ahh, that's right! Thank you @lukewalczak ! |
|
I have tested it on production build yesterday on Android ( APK 44756 ) and be able to transfer custom bg color and text color to Button on mobile.
It was bring by latest sync with master ? If not it may be something else causing this issue according to above |
|
Colors seem to be fixed in the latest test build @lukewalczak . Nice work! |
Description
Fixes: wordpress-mobile/gutenberg-mobile#1790
Ref to gb-mobile: wordpress-mobile/gutenberg-mobile#1933
Points to achieve:
ButtonADD BLOCK HERElineHow has this been tested?
Test cases can be found here
Screenshots
Types of changes
Checklist: