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

Jump to bottom

Feat: Re-structure theme styles and simplify ThemeStylesProvider #27324

Merged
merged 9 commits into from
Sep 14, 2023
Merged

Feat: Re-structure theme styles and simplify ThemeStylesProvider #27324

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
7c8ffcf
feat: improve how we use static styles in ThemeStylesProvider
Sep 13, 2023
7276e6d
fix: rename ThemeStylesContext
Sep 13, 2023
e3ce763
fix: lint errors and added TODOs for after theme switching migration
Sep 13, 2023
7cd0eeb
fix: ignore eslint instead of casting
Sep 13, 2023
97a3fe7
fix: cannot edit money request in offline mode
tienifr Sep 12, 2023
e5e697b
fix: add GH issue link to TODO comments
Sep 13, 2023
263a611
fix: add one more comment
Sep 13, 2023
cc62152
fix: update comment
Sep 13, 2023
068556a
fix: remove weird change
Sep 13, 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
2 changes: 1 addition & 1 deletion src/libs/ReportUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2627,7 +2627,7 @@ function buildTransactionThread(reportAction, moneyRequestReportID) {
participantAccountIDs,
getTransactionReportName(reportAction),
'',
lodashGet(getReport(reportAction.reportID), 'policyID', CONST.POLICY.OWNER_EMAIL_FAKE),
chrispader marked this conversation as resolved.
Show resolved Hide resolved
lodashGet(getReport(moneyRequestReportID), 'policyID', CONST.POLICY.OWNER_EMAIL_FAKE),
CONST.POLICY.OWNER_ACCOUNT_ID_FAKE,
false,
'',
Expand Down
21 changes: 21 additions & 0 deletions src/styles/StyleUtils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -259,9 +259,12 @@ function getSafeAreaMargins(insets?: EdgeInsets): ViewStyle | CSSProperties {

function getZoomCursorStyle(isZoomed: boolean, isDragging: boolean): ViewStyle | CSSProperties {
if (!isZoomed) {
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return styles.cursorZoomIn;
}

// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return isDragging ? styles.cursorGrabbing : styles.cursorZoomOut;
}

Expand Down Expand Up @@ -336,12 +339,16 @@ function getWidthStyle(width: number): ViewStyle | CSSProperties {
*/
function getAutoGrowHeightInputStyle(textInputHeight: number, maxHeight: number): ViewStyle | CSSProperties {
if (textInputHeight > maxHeight) {
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return {
...styles.pr0,
...styles.overflowAuto,
};
}

// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return {
...styles.pr0,
...styles.overflowHidden,
Expand Down Expand Up @@ -439,11 +446,17 @@ function getBackgroundColorWithOpacityStyle(backgroundColor: string, opacity: nu
function getBadgeColorStyle(success: boolean, error: boolean, isPressed = false, isAdHoc = false): ViewStyle | CSSProperties {
if (success) {
if (isAdHoc) {
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return isPressed ? styles.badgeAdHocSuccessPressed : styles.badgeAdHocSuccess;
}
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return isPressed ? styles.badgeSuccessPressed : styles.badgeSuccess;
}
if (error) {
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return isPressed ? styles.badgeDangerPressed : styles.badgeDanger;
}
return {};
Expand Down Expand Up @@ -593,6 +606,8 @@ function getLoginPagePromoStyle(): ViewStyle | CSSProperties {
* Generate the styles for the ReportActionItem wrapper view.
*/
function getReportActionItemStyle(isHovered = false, isLoading = false): ViewStyle | CSSProperties {
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return {
display: 'flex',
justifyContent: 'space-between',
Expand All @@ -609,6 +624,8 @@ function getReportActionItemStyle(isHovered = false, isLoading = false): ViewSty
* Generate the wrapper styles for the mini ReportActionContextMenu.
*/
function getMiniReportActionContextMenuWrapperStyle(isReportActionItemGrouped: boolean): ViewStyle | CSSProperties {
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return {
...(isReportActionItemGrouped ? positioning.tn8 : positioning.tn4),
...positioning.r4,
Expand Down Expand Up @@ -1043,6 +1060,8 @@ function displayIfTrue(condition: boolean): ViewStyle | CSSProperties {

function getGoogleListViewStyle(shouldDisplayBorder: boolean): ViewStyle | CSSProperties {
if (shouldDisplayBorder) {
// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return {
...styles.borderTopRounded,
...styles.borderBottomRounded,
Expand Down Expand Up @@ -1150,6 +1169,8 @@ function getDisabledLinkStyles(isDisabled = false): ViewStyle | CSSProperties {
...cursor.cursorDisabled,
};

// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return {
...styles.link,
...(isDisabled ? disabledLinkStyles : {}),
Expand Down
25 changes: 7 additions & 18 deletions src/styles/ThemeStylesProvider.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,8 +1,11 @@
/* eslint-disable react/jsx-props-no-spreading */
import React, {useMemo} from 'react';
import useTheme from './themes/useTheme';
import StylesContext from './ThemeStylesContext';
import defaultStyles from './styles';
import ThemeStylesContext from './ThemeStylesContext';
// TODO: Rename this to "styles" once the app is migrated to theme switching hooks and HOCs
import {stylesGenerator as stylesUntyped} from './styles';

const styles = stylesUntyped as (theme: Record<string, string>) => Record<string, unknown>;

type ThemeStylesProviderProps = {
children: React.ReactNode;
Expand All @@ -11,23 +14,9 @@ type ThemeStylesProviderProps = {
function ThemeStylesProvider({children}: ThemeStylesProviderProps) {
const theme = useTheme();

const appContentStyle = useMemo(
() => ({
...defaultStyles.appContent,
backgroundColor: theme.appBG,
}),
[theme.appBG],
);

const styles = useMemo(
() => ({
...defaultStyles,
appContent: appContentStyle,
}),
[appContentStyle],
);
const themeStyles = useMemo(() => styles(theme), [theme]);

return <StylesContext.Provider value={styles}>{children}</StylesContext.Provider>;
return <ThemeStylesContext.Provider value={themeStyles}>{children}</ThemeStylesContext.Provider>;
}

ThemeStylesProvider.displayName = 'ThemeStylesProvider';
Expand Down
2 changes: 2 additions & 0 deletions src/styles/getReportActionContextMenuStyles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,8 @@ function getReportActionContextMenuStyles(isMini: boolean, isSmallScreenWidth: b
return miniWrapperStyle;
}

// TODO: Remove this "eslint-disable-next" once the theme switching migration is done and styles are fully typed (GH Issue: https://github.com/Expensify/App/issues/27337)
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return [
...bigWrapperStyle,

Expand Down
Loading
Loading