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: appbar content title container adjustments #3596

Merged
merged 3 commits into from
Jan 23, 2023
Merged

Conversation

lukewalczak
Copy link
Member

Summary

PR introduces adjustments for handling the title prop in Appbar.Content to wrap only the title string within the default Text component. Other types passed into title are rendered directly in the View container.

Related issue

Test plan

Added unit tests.

@github-actions
Copy link

The mobile version of example app from this branch is ready! You can see it here.

Copy link
Member

@krozniata krozniata left a comment

Choose a reason for hiding this comment

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

LGTM!
It would be nice to add example to Usage using component as a title (maybe the one from tests)

@gedu
Copy link
Contributor

gedu commented Jan 18, 2023

if title is a string using the titleStyle doesn't make sense, what do you think to split the type into something like:

type TitleString = {
  /**
   * Text for the title.
   */
  title: string;
  titleStyle?:  StyleProp<TextStyle>;
};

type TitleElement = {
  /**
   * Element for the title.
   */
  title: React.ReactElement;
  /**
   * Style for the title.
   */
  titleStyle?: never;
};

// remove title and titleStyle from Props
export type Props = $RemoveChildren<typeof View> & { ... } & (TitleElement | TitleString);

So if you try to add title as string and titleStyle, TS warns you.

@callstack-bot
Copy link

callstack-bot commented Jan 23, 2023

Hey @lukewalczak, thank you for your pull request 🤗. The documentation from this branch can be viewed here.

@lukewalczak lukewalczak merged commit eece348 into main Jan 23, 2023
@lukewalczak lukewalczak deleted the fix/appbar-content branch January 23, 2023 13:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants