Skip to content
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

fix: inverted list padding #668

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

lpatrun
Copy link

@lpatrun lpatrun commented Oct 30, 2024

📜 Description

I noticed the padding is incorrectly added if the Flatlist/Flashlist is inverted. Also, I have noticed that because of the bottom tab navigation, the offset when the keyboard is opened is not correctly calculated and the bottom tab height should be deducted.

💡 Motivation and Context

This problem fixes padding on inverted flatlist/flashlist if the keyboard is opened.

📢 Changelog

JS

  • added inverted handling
  • added property to be able to adjust the offset when the keyboard is opened

🤔 How Has This Been Tested?

It was tested in the app.

@kirillzyusko
Copy link
Owner

@lpatrun may I also ask you to give a sample of a layout so that I can test it in example app?

@kirillzyusko kirillzyusko self-assigned this Oct 30, 2024
@kirillzyusko kirillzyusko added enhancement New feature or request KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component labels Oct 30, 2024
@lpatrun
Copy link
Author

lpatrun commented Oct 30, 2024

@kirillzyusko I will try your suggestion and get back to you. Thank you for your promptness

@@ -38,6 +38,10 @@ export type KeyboardAwareScrollViewProps = {
enabled?: boolean;
/** Adjusting the bottom spacing of KeyboardAwareScrollView. Default is `0` */
extraKeyboardSpace?: number;
/** Used if inverted Flatlist/Flashlist is being used */
inverted?: boolean;
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this property can be derived from style property?

style?.transform?.some(({scaleY}) => scaleY === -1);

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe I am missing how to call style, but when I call it as is in the component, I get

"Cannot find name 'style'."

and in rest.style I have only
"style": {"backgroundColor": "#FFFFFF", "flex": 1}} so ni scaleY.

But this inverted prop should be ok because it is a Flatlist/Flashlist prop. Right?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lpatrun can you show me how do you use FlatList and KeyboardAwareScrollView?

I assumed it would be something like that:

<FlatList
  data={[]}
  renderItem={() => <></>}
  inverted
  renderScrollComponent={(props) => <KeyboardAwareScrollView {...props} />}
/>

And here (if I understand correctly) props.style should contain transform property (it should be injected by FlatList). Or no? 🙈

Copy link
Author

@lpatrun lpatrun Oct 30, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the code from the app

 <FlatList
   contentContainerStyle={styles.dataStyles}
   data={[...messages].reverse()}
   inverted
   renderItem={({ item }) => renderChatElement(item)}
   ItemSeparatorComponent={() => <SpacingComponent val={4} />}
   renderScrollComponent={props => <KeyboardAwareScrollView {...props} />}
   ListHeaderComponent={() => (isLoading ? <Loading /> : <></>)}
/>

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @lpatrun

I'll check it today! ❤️

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @lpatrun

I added this code to KeyboardAwareScrollView (it has TS errors):

    const inverted = Array.isArray(rest.style)
      ? rest.style?.[0]?.transform[0]?.scaleY === -1
      : false;

    console.log("rest", rest.style, inverted);

And this code I used for testing:

    <FlatList
      // contentContainerStyle={{flex: 1}}
      // style={{flexGrow: 1}}
      data={[1]}
      inverted
      renderItem={({ item }) => <></>}
      renderScrollComponent={props => <KeyboardAwareScrollView {...props} />}
  />

And it seems to work fine:

image

Can you check on your side again?

@kirillzyusko
Copy link
Owner

@lpatrun I also merged some PRs into main and I think they can produce a conflict with your PR. May I ask you to merge/rebase your branch onto latest main?

@kirillzyusko
Copy link
Owner

@lpatrun what is the status of this PR? Are you going to finish the changes that I requested or not? 👀

@lpatrun
Copy link
Author

lpatrun commented Dec 13, 2024

@kirillzyusko my apologies, I will finish it

@lpatrun
Copy link
Author

lpatrun commented Dec 13, 2024

@kirillzyusko I made changes and I have tested with and without the inverted prop.

? {
// animations become choppy when scrolling to the end of the `ScrollView` (when the last input is focused)
// this happens because the layout recalculates on every frame. To avoid this we slightly increase padding
// by `+1`. In this way we assure, that `scrollTo` will never scroll to the end, because it uses interpolation
// from 0 to `keyboardHeight`, and here our padding is `keyboardHeight + 1`. It allows us not to re-run layout
// re-calculation on every animation frame and it helps to achieve smooth animation.
// see: https://github.com/kirillzyusko/react-native-keyboard-controller/pull/342
paddingBottom: currentKeyboardFrameHeight.value + 1,
paddingBottom: currentKeyboardFrameHeight.value + invertedOffset + 1,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why in case of inverted we do need to remove extraKeyboardSpace?

@kirillzyusko
Copy link
Owner

I approved to see if CI passes or not 👀

@kirillzyusko
Copy link
Owner

@lpatrun TS is failing 😔

@lpatrun
Copy link
Author

lpatrun commented Dec 15, 2024

@kirillzyusko how about now?

// re-calculation on every animation frame and it helps to achieve smooth animation.
// see: https://github.com/kirillzyusko/react-native-keyboard-controller/pull/342
paddingBottom:
currentKeyboardFrameHeight.value + invertedOffset + 1,
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Still a question about negative extraKeyboardSpace and 0 otherwise 🤔

Why do we need to remove extraKeyboardSpace in case of inverted value?

Comment on lines +116 to +121
const restStyle = (Array.isArray(rest.style) && rest.style?.[0]) || {};
const scaleStyle =
"transform" in restStyle &&
(restStyle?.transform?.[0] as { scale?: number });
const inverted = (scaleStyle && scaleStyle?.scale === -1) || false;

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ideally I would create a function isInverted (should be a pure function), I think you can put it in utils

And I would add useMemo here to avoid expensive checks 🤔

So the code should be:

const inverted = useMemo(() => isInverted(rest.style), [rest.style]);

What do you think?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@lpatrun just a friendly ping 👋

Copy link
Contributor

📊 Package size report

Current size Target Size Difference
165756 bytes 164428 bytes 1328 bytes 📈

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request KeyboardAwareScrollView 📜 Anything related to KeyboardAwareScrollView component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants