prevent overlap of clear button, fix font, reduce space on recent statuses #8322

Willyfrog · 2024-11-08T11:12:04Z

Summary

clear button on long statuses was being drawn on top of the text
not using same font as rest of the app
recent status were having huge space between text and clear button

Ticket Link

MM-61129

Checklist

Added or updated unit tests (required for all new features)
Has UI changes
Includes text changes and localization file updates
Have tested against the 5 core themes to ensure consistency between them.
Have run E2E tests by adding label E2E iOS tests for PR.

Device Information

This PR was tested on: android and ios simulators

Screenshots

Release Note

fix UI for editing custom status

…tuses

larkox

Minor comments, but approving preemptively.

app/screens/custom_status/components/custom_status_input.tsx

larkox · 2024-11-08T12:55:08Z

app/screens/custom_status/components/custom_status_input.tsx

@@ -83,11 +80,11 @@ const CustomStatusInput = ({emoji, isStatusSet, onChangeText, onClearHandle, onO
                    style={style.input}
                    secureTextEntry={false}
                    underlineColorAndroid='transparent'
+                    multiline={true}


Do we want custom status to be multiline? Or am I missing something?

no, just the display of it to wrap around and use multiple lines instead of scrolling

I think that adding multiline allows you to add line breaks, which is probably not desired. I wonder if we should reconsider how these input work visually. But 0/5 on whether it should be in this PR or not.

I couldn't add line breaks into the status using the iOS simulator.
I'll double check with a real device to make sure

even if the iOS simulator does not allow you to add line breaks, the Android one does.

I cannot seem to find a way to wrap the text in the input in multiple lines without having multi-line enabled.

Perhaps what you can do is before submitting the text of the status, you could just modify it to remove all line breaks if there are any and potentially replace them with spaces.

it seems external keyboards on android are wired differently,
added a check onTextChange to remove any new lines

Sadly this generates a flicker, and looks like everyone is having the same problem.

Not there.. before sendin the text to the action or in the action itself

moved to the reducer action

enahum

approving as not to block as the other review is already addressing the same concern. I did provide a potential solution, so If you apply it, do ask for my review again.

enahum · 2024-11-09T00:32:25Z

app/screens/custom_status/components/custom_status_input.tsx

@@ -83,11 +80,11 @@ const CustomStatusInput = ({emoji, isStatusSet, onChangeText, onClearHandle, onO
                    style={style.input}
                    secureTextEntry={false}
                    underlineColorAndroid='transparent'
+                    multiline={true}


even if the iOS simulator does not allow you to add line breaks, the Android one does.

I cannot seem to find a way to wrap the text in the input in multiple lines without having multi-line enabled.

Perhaps what you can do is before submitting the text of the status, you could just modify it to remove all line breaks if there are any and potentially replace them with spaces.

enahum · 2024-11-11T16:52:32Z

app/screens/custom_status/custom_status.tsx

@@ -128,7 +128,7 @@ function reducer(state: NewStatusType, action: {
            return state;
        }
        case 'text':
-            return {...state, text: action.value};
+            return {...state, text: action.value?.replace('\n', '')};


should this be a space instead ?

Suggested change

return {...state, text: action.value?.replace('\n', '')};

return {...state, text: action.value?.replace('\n', ' ')};

That intentional, in my head it made more sense to ignore the input rather than having a space, but I'm 1/5 and easily convinced as I don't know what the right behavior could be

Willyfrog · 2024-11-14T10:01:35Z

@larkox gentle reminder to re-review :)

lindy65

Thanks @Willyfrog - tested on Android and iOS and this is fixed

prevent overlap of clear button, fix font, reduce space on recent sta…

76d9528

…tuses

mm-cloud-bot added the release-note label Nov 8, 2024

Willyfrog added the E2E iOS tests for PR Run iOS E2E Detox tests label Nov 8, 2024

Willyfrog requested review from larkox and rahimrahman November 8, 2024 11:22

Willyfrog added the 2: Dev Review Requires review by a core commiter label Nov 8, 2024

github-actions bot removed the E2E iOS tests for PR Run iOS E2E Detox tests label Nov 8, 2024

larkox approved these changes Nov 8, 2024

View reviewed changes

remove font size

e2085d9

enahum self-requested a review November 8, 2024 23:07

enahum approved these changes Nov 9, 2024

View reviewed changes

prevent newlines on android external keyboard

31df4df

Willyfrog requested review from enahum and larkox November 11, 2024 15:21

move to reducer action

a132703

enahum requested changes Nov 11, 2024

View reviewed changes

change newline behaviour to space

855e5a2

Willyfrog requested a review from enahum November 12, 2024 09:42

enahum approved these changes Nov 12, 2024

View reviewed changes

larkox approved these changes Nov 14, 2024

View reviewed changes

Willyfrog added 3: QA Review Requires review by a QA tester and removed 2: Dev Review Requires review by a core commiter labels Nov 14, 2024

Willyfrog requested review from lindy65 and removed request for rahimrahman November 14, 2024 10:38

lindy65 added the Build Apps for PR Build the mobile app for iOS and Android to test label Nov 14, 2024

lindy65 approved these changes Nov 14, 2024

View reviewed changes

lindy65 added 4: Reviews Complete All reviewers have approved the pull request and removed Build Apps for PR Build the mobile app for iOS and Android to test 3: QA Review Requires review by a QA tester labels Nov 14, 2024

lindy65 added the QA Review Done label Nov 14, 2024

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

prevent overlap of clear button, fix font, reduce space on recent statuses #8322

prevent overlap of clear button, fix font, reduce space on recent statuses #8322

Willyfrog commented Nov 8, 2024 •

edited

Loading

larkox left a comment

larkox Nov 8, 2024

Willyfrog Nov 8, 2024

larkox Nov 8, 2024

Willyfrog Nov 8, 2024

enahum Nov 9, 2024

Willyfrog Nov 11, 2024

enahum Nov 11, 2024

Willyfrog Nov 11, 2024

enahum left a comment

enahum Nov 9, 2024

enahum Nov 11, 2024

Willyfrog Nov 11, 2024

Willyfrog commented Nov 14, 2024

lindy65 left a comment

	return {...state, text: action.value?.replace('\n', '')};
	return {...state, text: action.value?.replace('\n', ' ')};

prevent overlap of clear button, fix font, reduce space on recent statuses #8322

Are you sure you want to change the base?

prevent overlap of clear button, fix font, reduce space on recent statuses #8322

Conversation

Willyfrog commented Nov 8, 2024 • edited Loading

Summary

Ticket Link

Checklist

Device Information

Screenshots

Release Note

larkox left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enahum left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Willyfrog commented Nov 14, 2024

lindy65 left a comment

Choose a reason for hiding this comment

Willyfrog commented Nov 8, 2024 •

edited

Loading