Conversation
etoledom
left a comment
There was a problem hiding this comment.
Hey @SergioEstevao - Thanks for taking care of this feature! 🎉
I found some weird behavior while testing it on iOS:
- Changing settings won't always be reflected visually on the block
- At some point the block format would break
There's a gif showing this issue:
It's interesting that some times the visual changes would work according to the settings. For example:
- Create the ordered list with some elements.
- Change settings to reversed (It won't show the changes visually in the list).
- Go to HTML mode and back.
- The block is re-rendered with the correct list order (reversed).
- Select the block without making the keyboard active (tapping at the very edge).
- Select the block settings and change properties.
- It will refresh the block visually to reflect the changes ✅
- Select the block making the keyboard active and change some settings.
- It won't refresh the block visually ❌
One possible enhancement could be to show the numeric pad to set the Start Value setting.
I saw that TextControl has a type prop that is set to number on this setting, but it's not being handled properly in our native TextControl version. I think that if we pass the corresponding keyboardType to the Cell component it should work.
I gave it a pass on Android but I think it's not ready yet, right?
|
@etoledom Thanks for the great review! Two great catches, I updated the code to address those issues, can you please give it another look? |
etoledom
left a comment
There was a problem hiding this comment.
Thank you @SergioEstevao for the update, it's working great now!
All the issues pointed out previously are now fixed 🎉
Not sure if we should wait for the Android implementation or we could have a platform check to ship on iOS ahead of time.
|
@marecar3 I tested the lists in Android and I see one bug, if you choose a reversed list without specifying a start value, the values are going from top value to 0. For example if you have 5 items we see: While the correct should be: |
# Conflicts: # gutenberg
Hey @SergioEstevao, thanks for the feedback. |
|
Hey @SergioEstevao, you can do another iteration of testing :) Thanks! |
|
@marecar3 Working great now! Thanks for this! Do you need to merge anything in Aztec Android before I merge this? |
# Conflicts: # react-native-aztec/ios/Cartfile # react-native-aztec/ios/Cartfile.resolved
Fixes #1304
To test:
Update release notes:
RELEASE-NOTES.txt.