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.

Sign up for GitHub

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

[No QA] [TS migration] Migrate 'getNavigationModalCardStyles' style to TypeScript #25923

Merged
merged 3 commits into from
Aug 25, 2023
Merged

[No QA] [TS migration] Migrate 'getNavigationModalCardStyles' style to TypeScript #25923

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
041aeae
[TS migration] Migrate 'getNavigationModalCardStyles' style to TypeSc…
BartoszGrajdek Aug 25, 2023
c39bb2f
Fix getBaseNavigationModal naming issue
BartoszGrajdek Aug 25, 2023
6b1a41e
Simplify types
BartoszGrajdek Aug 25, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 4 additions & 1 deletion ...tyles/getBaseNavigationModalCardStyles.js → ...tyles/getBaseNavigationModalCardStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import variables from '../variables';
import GetNavigationModalCardStyles from './types';

export default ({isSmallScreenWidth}) => ({
const getBaseNavigationModalCardStyles: GetNavigationModalCardStyles = ({isSmallScreenWidth}) => ({
position: 'absolute',
top: 0,
right: 0,
width: isSmallScreenWidth ? '100%' : variables.sideBarWidth,
backgroundColor: 'transparent',
height: '100%',
});

export default getBaseNavigationModalCardStyles;
0 ...les/getNavigationModalCardStyles/index.js → ...les/getNavigationModalCardStyles/index.ts
View file
Open in desktop
File renamed without changes.
5 changes: 4 additions & 1 deletion ...avigationModalCardStyles/index.website.js → ...avigationModalCardStyles/index.website.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import getBaseNavigationModalCardStyles from './getBaseNavigationModalCardStyles';
import GetNavigationModalCardStyles from './types';

export default ({isSmallScreenWidth}) => ({
const getNavigationModalCardStyles: GetNavigationModalCardStyles = ({isSmallScreenWidth}) => ({
...getBaseNavigationModalCardStyles({isSmallScreenWidth}),

// position: fixed is set instead of position absolute to workaround Safari known issues of updating heights in DOM.
Expand All @@ -10,3 +11,5 @@ export default ({isSmallScreenWidth}) => ({
// https://github.com/Expensify/App/issues/20709
position: 'fixed',
});

export default getNavigationModalCardStyles;
10 changes: 10 additions & 0 deletions src/styles/getNavigationModalCardStyles/types.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
import {CSSProperties} from 'react';
import {ViewStyle} from 'react-native';
import {Merge} from 'type-fest';

type GetNavigationModalCardStylesParams = {isSmallScreenWidth: number};
type GetNavigationModalCardStylesKeys = 'position' | 'top' | 'right' | 'width' | 'backgroundColor' | 'height';

type GetNavigationModalCardStyles = ({isSmallScreenWidth}: GetNavigationModalCardStylesParams) => Merge<Pick<ViewStyle, GetNavigationModalCardStylesKeys>, Pick<CSSProperties, 'position'>>;
Copy link
Contributor

Choose a reason for hiding this comment

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

Hi @BartoszGrajdek , for this one I was going to suggest to relax the types a little bit, but I'm having this strange TS error.

// types.ts
import {CSSProperties} from 'react';
import {ViewStyle} from 'react-native';

type GetNavigationModalCardStylesParams = {isSmallScreenWidth: number};

type GetNavigationModalCardStyles = (params: GetNavigationModalCardStylesParams) => ViewStyle | CSSProperties;

export default GetNavigationModalCardStyles;

// index.website.ts
import getBaseNavigationModalCardStyles from './getBaseNavigationModalCardStyles';
import GetNavigationModalCardStyles from './types';

const getNavigationModalCardStyles: GetNavigationModalCardStyles = ({isSmallScreenWidth}) => ({
    ...getBaseNavigationModalCardStyles({isSmallScreenWidth}),

    // position: fixed is set instead of position absolute to workaround Safari known issues of updating heights in DOM.
    // Safari issues:
    // https://github.com/Expensify/App/issues/12005
    // https://github.com/Expensify/App/issues/17824
    // https://github.com/Expensify/App/issues/20709
    position: 'fixed',
});

export default getNavigationModalCardStyles;
Screenshot 2023-08-25 at 13 58 35

Could you check if you have the same problem?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I do not have this problem

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll make the types less complicated first, then we'll see if you still have this error

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for the feedback @BartoszGrajdek. Do you agree with my suggestion?

Copy link
Contributor

Choose a reason for hiding this comment

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

then we'll see if you still have this error

I think you don't need to worry about this, it's the second time today I had similar issue with TS where others not, is something in my machine.

Copy link
Contributor Author

@BartoszGrajdek BartoszGrajdek Aug 25, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

I think that's caused by position: 'fixed' which is confusing to TS

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, or if you remove ...getBaseNavigationModalCardStyles({isSmallScreenWidth}), also works. I honestly don't know why this is happening, if we don't have a solution for this error I would suggest to you to proceed with your typings.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

What do you think about this? @fabioh8010

import {CSSProperties} from 'react';
import {ViewStyle} from 'react-native';
import {Merge} from 'type-fest';

type GetNavigationModalCardStylesParams = {isSmallScreenWidth: number};

type GetNavigationModalCardStyles = ({isSmallScreenWidth}: GetNavigationModalCardStylesParams) => Merge<ViewStyle, Pick<CSSProperties, 'position'>>;

export default GetNavigationModalCardStyles;

Copy link
Contributor

Choose a reason for hiding this comment

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

You can simplify the parameter

type GetNavigationModalCardStyles = (params: GetNavigationModalCardStylesParams) => Merge<ViewStyle, Pick<CSSProperties, 'position'>>;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, added these changes. It's simplified now


export default GetNavigationModalCardStyles;