Size Change: -20 B (0%)

Total Size: 842 kB

_{compressed-size-action}

Found a new crash on this PR (no crash on gb-mobile develop) tapping the toolbar button to split a Group block into separate blocks.
Steps:

1. Add a group block with one or more blocks
2. Tap Arrow to select entire Group block
3. Tap toolbar button to split group block into separate blocks
4. Crash, see screen shot below

thanks for reporting ! Will check that today

I also noticed that it crash after try to remove any block. Already fixed that.

The cause was that after ungroup (or delete) block breadcrumbs can not read block icon.

I have also add support for RTL according to this comment

@pinarol No, I meant if we can solve in a way that allows us to place the toolbar on the bottom edge, like iOS. Although as I mentioned above, I'm comfortable with differing positions for now, because it's a real improvement over what we currently have.

Actually the placement at the bottom isn’t the problem but covering the caret is. I don’t even know if there’s a way on Android to customize the scroll amount relative to caret to prevent the floating toolbar from covering the caret. I wouldn’t bet on it to happen soon considering the amount of work it took on iOS. But i am no Android expert so I’d like to get some opinions from @hypest @maxme. One thing i know for sure is, if we make the floating toolbar take up the full-width on top of the current toolbar then there will be no problem.

Actually the placement at the bottom isn’t the problem but covering the caret is.

Yea, I just mean the general positioning of the UI component (bottom vs. top) — if/when we were able to address that on Android — but it sounds like that's not likely. So I am okay with shipping Android top-aligned, then we can iterate if it causes problems. I don't think top-aligned is necessarily a terrible option, just less discoverable and less reachable than bottom-aligned.

One thing i know for sure is, if we make the floating toolbar take up the full-width on top of the current toolbar then there will be no problem.

I would still prefer a top-aligned truly "floating" toolbar over a docked full-width toolbar, but if this performs poorly I do have some ideas for how a full-width toolbar would look.

Blockers

• There are some small inconsistencies with scrolling position when selecting blocks but I think those aren't specific to the toolbar. Android especially doesn't seem to handle scroll position quite as smoothly. If these are quick refinements and related to this work, let's solve now — otherwise don't consider a blocker.

As Pinar mentioned there is differences between Android and iOS in autoscrolling and I think it's not straightforward to change that at this point.

• [iOS] One thing I think we probably should do, however, is add a small bit of additional spacing between the selected block and the toolbar when the block is nearby. By this, I mean adjust the y-position when a new block is selected so that there is at least ~8px spacing between the bottom-positioned floatingToolbar and the bottom of the selected block. (Example)

Fixed!

• Not necessarily a "blocker" per sé, but I would like to adjust the background-color of the floating toolbar to be the same color as the block toolbar. If my color picker in Figma is accurate, that looks like #2e2e2e — if there's a variable that represents that, let's use that to be consistent.

gray-500 was used. I switched to $background-dark-elevated (equivalent of #2e2e2e)

See below screen for light and dark mode

Non-blockers

• Can we scroll the canvas to the associated block when the selected block part of the breadcrumbs is tapped?

I have started to investigate how to implement that according to our convo on Slack but do not have worked version because of focusing on behaviour with text blocks.

• It'd be nice to apply some subtle transitions, if possible:
  - Upon entry: 300ms transition using easeOut, which would fade in and translate the y-position ~4-8px upward
  - Upon entry: quick 50ms transition using easeIn, which would fade out and translate the y-position ~4-8px downward
  - Here's what the animation would look like together

It's doable and I think it's not a big deal. Do we want to include that feature in this PR @pinarol ?

Others

Maybe unrelated, but when a new block is inserted on to the canvas, the scroll position does not adjust (example). This happens on both platforms.

I don't think it's related to FloatingToolbar itself. It's rather something with autoscroll

Can we scroll the canvas to the associated block when the selected block part of the breadcrumbs is tapped?

I have started to investigate how to implement that according to our convo on Slack but do not have worked version because of focusing on behaviour with text blocks.

This was tried as a part of this PR in a slightly different way(the aim was to scroll to the newly created block but the mechanism needed was the same) and it was reverted because of multiple problems. This is not a straightforward thing to do, so I suggest let's not block this PR for that. Buttons block would really benefit from having this version of the FloatingToolbar because the current one pushes the button down and it looks jumpy. Same for Columns when stacked horizontally.

It'd be nice to apply some subtle transitions, if possible:

• Upon entry: 300ms transition using easeOut, which would fade in and translate the y-position ~4-8px upward
• Upon entry: quick 50ms transition using easeIn, which would fade out and translate the y-position ~4-8px downward
• Here's what the animation would look like together

It's doable and I think it's not a big deal. Do we want to include that feature in this PR @pinarol ?

I suggest starting implementing this right after this PR considering that this was listed below non-blockers. I really prefer to have this PR merged asap to get rid of the jumpiness of individual Button blocks when stacked horizontally with other Buttons.

I suggest starting implementing this right after this PR considering that this was listed below non-blockers. I really prefer to have this PR merged asap to get rid of the jumpiness of individual Button blocks when stacked horizontally with other Buttons.

Ok so I will prepare builds after adjustments pointed as a blockers. After it gets Thomas approve as well as code review approve from @dratwas it can be merge

Refreshed build are here:
iOS: IPA 27394
Andorid: APK 46171

@iamthomasbishop it also includes merged Buttons so you can check how FloatingToolbar works with them as well

@jbinda This is feeling pretty solid! The visual changes look good, and I don't think any of the additional requests I had should stop this from iteration shipping (the animations, scrolling to the selected block, etc). Great work!!

@jbinda One tiny thing: I noticed while testing on Android that the toolbar background isn't using the updated $background-dark-elevated color. I tried un-installing and re-installing to make sure I wasn't on the previous build, but I'm on the updated one afaik.

@iamthomasbishop ok I will check that and fix

edited

I have checked o the Emulator and it seems it has proper color:

rgb(46 ,46, 46) is equal to #2e2e2e

I have also check production build on device and it looks like this:

Can you also post some screenshot from your side ?

Oh, that's odd. Maybe I have the wrong build on Android 🤔 I am seeing this on Android:

Light	Dark

As discussed on Slack I have also spot that issue on different Emulator with Android 9 (which does not support dar theme) and be able to fix that so probably will prepare fresh build (only for Android) to confirm on your side.

edited

Build for Android: APK 46462

Thank you al for checking this !