-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
[RNMobile] Search Block: Implement long button text handling #30855
Conversation
- Re-evaluate during orientation changes as well
cc: @iamthomasbishop |
Size Change: +2.13 kB (0%) Total Size: 1.46 MB
ℹ️ View Unchanged
|
@geriux I've made the suggested changes. Thank you for the tip, that really helped to remove a lot of unnecessary code. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really great, @AmandaRiu. I definitely prefer this approach over the web's because it more gracefully accommodates long labels, but I worry that despite the layout looking like a cluster, folks will be a little confused when they see a mismatch between the layout in the mobile editor vs. the front-end output. I think this is one of those awkward cases where we have to decide whether we should:
- Align with the front-end output (and core) and have a...less-than-ideal UI in the editor in order to match with the output, or...
- Make it look nicer in the editor (like you have here) which will result in a diff between what the user is seeing in the editor and on the front-end of their site
As much as it pains me to say, I think it'd be better to align with styling of the output (at least in terms of layout) for the time being. However, I believe this approach would be beneficial everywhere, so we should propose it upstream to core Gutenberg. I understand this isn't ideal, but it might be worth considering for a long-term solution.
@kyleaparker what do you think? Is it worth diverging from the output in this case, wrt layout behavior?
Note: my assumption here is that the front-end result that we're producing uses the same exact markup that is applied using the web editor — if I'm wrong and we're applying our own set of styles to make the block behave as it does in the mobile editor, then by all means, let's roll with this 😃
Hi @iamthomasbishop 👋 This design is specific and custom to the mobile editor and was in the i2 designs for the mobile version of the search block. I believe @kyleaparker approached this much like we approach the columns block in the mobile editor which shows each column as a row if space is limited. Editing the columns on mobile will look differently than web which may have some drawbacks, but the experience and UX is so much better than if we tried to make everything on mobile look just like it would on the web. The Search block is due at the end of this sprint (tomorrow) and this PR is blocking the final PR that will put the Search block into production so I'd like to get a final yay or nay on this functionality if possible. Thank you for the review! |
I think it is! If you're editing your site on mobile we should make sure you have the most optimal user experience. We should definitely push for this change for the web because long button text alongside a search input looks a little broken right now. We should also push towards a front end that matches and provides the best mobile experience for site visitors. FWIW, I don't think too many people will deviate from the Search text so I imagine the number of people who see this layout will be small. |
@AmandaRiu To be clear, I don't think this is worth blocking the rest of the project — I would consider it somewhat of an edge case that will only be experienced if you have a super-long text label. With that said, I did want to discuss because there is a potential for a relatively large mismatch between what the user sees on the front-end vs. in the editor. Definitely not worth holding up the entire project for, but rather something we can decide and iterate on. I get where you're coming from and 100% agree that the interaction model here is better from a UX perspective inside the editor, but if the produced layout on the front-end is substantially different (in this case, worse) then it's bound to cause confusion (in this particular case where they have a very long label — again, probably an edge case). Regarding Columns, unless there's a new bug I'm not aware of, layouts that you build in the mobile editor should be laid out on the front-end in roughly the same way and utilize the same breakpoints that are provided by core GB. IIRC, when we worked on the initial versions of Columns we made the same sort of decision — to follow the same breakpoints and layout that exist on the web (for better or worse). |
@kyleaparker I agree w/ most of what you're saying; it does feel counter-intuitive to suggest that we not default to the absolute best authoring experience. 😄 I just wanted to voice my concern about the potential frustration a user might experience when they've added a long label in the editor (which adapts nicely as shown above), but sees that the result on the front-end is janky — but that's part of a much bigger conversation. Considering the scale of impact for better or worse is small in this case, I'm totally fine with whatever y'all think is best here! |
@kyleaparker @iamthomasbishop I appreciate the open discussion and thank you for all your input. I agree there is so much gray area here and it's a difficult and very important thing to be aware of so we can create the best possible experience for the user. That said, it sounds like we are keeping these changes so this PR is good to merge once approved. 👍 |
NOTE: Opened a ticket to have this same functionality ported to the mobile web version as well. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM. I tested this an iPhone SE and the Android emulator. Operated as expected, aside from one comment regarding the button width expanding while typing.
@AmandaRiu I did just run into an issue that may be worth investigating further. 😞 When (1) inserting the Search block while having (2) a text block focused and (3) using the block inserter search, an infinite focus loop is created. This does not occur if I do not have all three of those criteria met.
An infinite focus loop is created, which eventually would likely lead to a crash. This is not the first time I have seen an infinite loop. There are a few open issues — wordpress-mobile/gutenberg-mobile#953, wordpress-mobile/gutenberg-mobile#1696. However, this one was pretty easy for me to recreate. I have not yet tested this within the WPiOS and WPAndroid apps themselves, only the Demo editor app. I'm not sure if the issue would be better or worse in the actual apps. I'll try to find time help investigate, but it may be a bit before I can spend more time. RPReplay_Final1618531729.MP4 |
Thanks for the review @dcalhoun! I tried to replicate the focus issue but am not having any luck. I tried on an Android emulator, as well as my Pixel 4 running Android 11. What are the specs of the device you were using for testing? screen-20210415-204742.mp4 |
iPhone SE, iOS 14.4.2. Local Metro server running on this branch. I'll try to spend some time tomorrow investigating further. |
Regarding the loop problem, I created an issue #30562 for that (the search functionality is still in dev) |
Aha! Okay, I tested on my physical iPhone 11 running iOS v14.4.2 and can replicate this issue. It's not the search block however, it happens on any text block. Here's me testing with adding a paragraph block: screencapture-1618854323923.mp4 |
Gutenberg Mobile PR: wordpress-mobile/gutenberg-mobile#3210
Description
This PR implements the design for moving the button below the input element if the button text makes the button grow to more than 50% of the available width of the block:
Changes include the following:
Known issues/not included
maxWidth
is set appropriately but the text will still wrap before it fills the full width.How has this been tested?
Test the following scenarios on iOS simulator and Android emulator.
Long button text moves button below search input element
Long text button moves back inline with search input when icon only option enabled
Long button with button position set to "inside"
Long button recalculated on orientation change
Screenshots
Interacting with the UI
Screen.Recording.2021-04-14.at.6.24.16.PM.mov
Rotating on Android
android-rotate.mov
Rotating on iOS
ios-rotate.mov
Types of changes
Checklist:
*.native.js
files for terms that need renaming or removal).