react-native icon indicating copy to clipboard operation
react-native copied to clipboard

fix(Android) drawing additional empty line when 'textAlign' is set to 'justify'

Open coado opened this issue 1 year ago • 3 comments

Summary:

Fixes #46908

The justificationMode is not set for multiline text without unicode characters with known width on both architectures. This caused the issue of drawing additional empty line at the end of TextView because Yoga thought that text takes 5 lines and falsely calculated it's height.

Currently, on the old architecture, the justificationMode is set only on text that is not boring (contains unicode characters) with unknown width. I am not sure why is that, so I am opening this as a draft for now as I am still checking if it doesn't break anything.

Changelog:

[ANDROID] [FIXED] - fix generating empty line at the end of multiline text view when textAlign is set to justify

Test Plan:

I've tested on both architectures on repro provided in the issue.

coado avatar Oct 18 '24 13:10 coado

Fails
:no_entry_sign:

Danger failed to run dangerfile.js.

Error ReferenceError

validateChangelog is not defined
ReferenceError: validateChangelog is not defined
    at Object.<anonymous> (dangerfile.js:48:18)
    at Module._compile (node:internal/modules/cjs/loader:1364:14)
    at requireFromString (/home/runner/work/react-native/react-native/node_modules/require-from-string/index.js:28:4)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:161:68
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:52:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:33:53)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:27:71
    at new Promise (<anonymous>)
    at __awaiter (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:23:12)
    at runDangerfileEnvironment (/home/runner/work/react-native/react-native/node_modules/danger/distribution/runner/runners/inline.js:118:132)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:181:38
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:44:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:25:53)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:19:71
    at new Promise (<anonymous>)
    at __awaiter (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:15:12)
    at Object.executeRuntimeEnvironment (/home/runner/work/react-native/react-native/node_modules/danger/distribution/platforms/GitHub.js:144:88)
    at /home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:101:47
    at step (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:34:23)
    at Object.next (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:15:53)
    at fulfilled (/home/runner/work/react-native/react-native/node_modules/danger/distribution/commands/danger-runner.js:6:58)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

Dangerfile

43|     'This will require a manual import by a Facebook employee.';
44|   warn(`${title} - <i>${idea}</i>`);
45| }
46| 
47| // Provides advice if a test plan is missing.
--------------------^
48| const includesTestPlan =
49|   danger.github.pr.body &&
50|   danger.github.pr.body.toLowerCase().includes('## test plan');
51| if (!includesTestPlan && !isFromPhabricator) {

Generated by :no_entry_sign: dangerJS against 232ed3f4f25cefe4f2f52f934a63bedfc2bf7de0

react-native-bot avatar Oct 18 '24 13:10 react-native-bot

These three test cases enter all specified layout builders in createLayout (new arch) and measureSpannedText (old arch) and it seems to work

code
import {
  SafeAreaView,
  StyleSheet,
  Text,
} from 'react-native';

function App(): React.JSX.Element {
  return (
    <SafeAreaView style={{ backgroundColor: 'white', flex: 1, justifyContent: 'center'}}>

      {/* boring layout */}
      <Text style={[styles.textJustify, styles.containerWidth]}>
        asdasjand akjsnd snd jsdnjkdn js s
      </Text>

      {/* multiline, boring layout */}
      <Text style={[styles.textJustify, styles.containerWidth]}>
        asdasd dajdoij sdnk aosjn dojna s dnjasjdkn dnsjdnjnkdaajs sjdnjn sdjn
        jdns ssjkndjkansd s s s s s s s njnd sjkdnajdn sndjand sdjnaojdnsjdnsnd
        jsdn sdn a s kk sdnjsjdn jsnd sjdn dddd ajksnd jad nd a d sakjnd skjnd
        sjdnaj ksjand akjsnd snd jsdnjkdn js s
      </Text>

      {/* not boring, single line */}
      <Text style={[styles.textJustify]}>
      تخطيط الاختبارتخطيط الاختبارتخطيط الاختبرا تخطيط تخطيط تخطيط تخطيط ا ا
      </Text>

    </SafeAreaView>
  );
}

const styles = StyleSheet.create({
  containerWidth: {
    width: 400,
  },
  textJustify: {
    backgroundColor: 'red',
    textAlign: 'justify',
    margin: 16,
  },
});

export default App;

coado avatar Oct 22 '24 10:10 coado

@NickGerleman has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

facebook-github-bot avatar Oct 25 '24 23:10 facebook-github-bot

@NickGerleman merged this pull request in facebook/react-native@08e8f6adfdcdfec59002f34bb7acf35bb842b331.

facebook-github-bot avatar Oct 29 '24 21:10 facebook-github-bot

This pull request was successfully merged by @coado in 08e8f6adfdcdfec59002f34bb7acf35bb842b331

When will my fix make it into a release? | How to file a pick request?

react-native-bot avatar Oct 29 '24 21:10 react-native-bot