Skip to content

Commit

Permalink
Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
fabioh8010 committed Sep 20, 2023
1 parent dc0722d commit 05a58d8
Show file tree
Hide file tree
Showing 7 changed files with 31 additions and 31 deletions.
42 changes: 21 additions & 21 deletions src/styles/StyleUtils.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import {Animated, FlexStyle, PressableStateCallbackType, TextStyle, ViewStyle} from 'react-native';
import {Animated, DimensionValue, PressableStateCallbackType, TextStyle, ViewStyle} from 'react-native';
import {EdgeInsets} from 'react-native-safe-area-context';
import {ValueOf} from 'type-fest';
import CONST from '../CONST';
Expand Down Expand Up @@ -272,7 +272,7 @@ function getZoomCursorStyle(isZoomed: boolean, isDragging: boolean): ViewStyle {
return isDragging ? styles.cursorGrabbing : styles.cursorZoomOut;
}

// NOTE: asserting some web style properties to a valid type, because isn't possible to augment them.
// NOTE: asserting some web style properties to a valid type, because it isn't possible to augment them.
function getZoomSizingStyle(
isZoomed: boolean,
imgWidth: number,
Expand All @@ -286,23 +286,23 @@ function getZoomSizingStyle(
if (isLoading || imgWidth === 0 || imgHeight === 0) {
return undefined;
}
const top = `${Math.max((containerHeight - imgHeight) / 2, 0)}px` as ViewStyle['top'];
const left = `${Math.max((containerWidth - imgWidth) / 2, 0)}px` as ViewStyle['left'];
const top = `${Math.max((containerHeight - imgHeight) / 2, 0)}px` as DimensionValue;
const left = `${Math.max((containerWidth - imgWidth) / 2, 0)}px` as DimensionValue;

// Return different size and offset style based on zoomScale and isZoom.
if (isZoomed) {
// When both width and height are smaller than container(modal) size, set the height by multiplying zoomScale if it is zoomed in.
if (zoomScale >= 1) {
return {
height: `${imgHeight * zoomScale}px` as FlexStyle['height'],
width: `${imgWidth * zoomScale}px` as FlexStyle['width'],
height: `${imgHeight * zoomScale}px` as DimensionValue,
width: `${imgWidth * zoomScale}px` as DimensionValue,
};
}

// If image height and width are bigger than container size, display image with original size because original size is bigger and position absolute.
return {
height: `${imgHeight}px` as FlexStyle['height'],
width: `${imgWidth}px` as FlexStyle['width'],
height: `${imgHeight}px` as DimensionValue,
width: `${imgWidth}px` as DimensionValue,
top,
left,
};
Expand All @@ -311,20 +311,20 @@ function getZoomSizingStyle(
// If image is not zoomed in and image size is smaller than container size, display with original size based on offset and position absolute.
if (zoomScale > 1) {
return {
height: `${imgHeight}px` as FlexStyle['height'],
width: `${imgWidth}px` as FlexStyle['width'],
height: `${imgHeight}px` as DimensionValue,
width: `${imgWidth}px` as DimensionValue,
top,
left,
};
}

// If image is bigger than container size, display full image in the screen with scaled size (fit by container size) and position absolute.
// top, left offset should be different when displaying long or wide image.
const scaledTop = `${Math.max((containerHeight - imgHeight * zoomScale) / 2, 0)}px` as ViewStyle['top'];
const scaledLeft = `${Math.max((containerWidth - imgWidth * zoomScale) / 2, 0)}px` as ViewStyle['left'];
const scaledTop = `${Math.max((containerHeight - imgHeight * zoomScale) / 2, 0)}px` as DimensionValue;
const scaledLeft = `${Math.max((containerWidth - imgWidth * zoomScale) / 2, 0)}px` as DimensionValue;
return {
height: `${imgHeight * zoomScale}px` as FlexStyle['height'],
width: `${imgWidth * zoomScale}px` as FlexStyle['width'],
height: `${imgHeight * zoomScale}px` as DimensionValue,
width: `${imgWidth * zoomScale}px` as DimensionValue,
top: scaledTop,
left: scaledLeft,
};
Expand Down Expand Up @@ -448,8 +448,8 @@ function getBackgroundColorWithOpacityStyle(backgroundColor: string, opacity: nu
/**
* Generate a style for the background color of the Badge
*/
function getBadgeColorStyle(success: boolean, error: boolean, isPressed = false, isAdHoc = false): ViewStyle {
if (success) {
function getBadgeColorStyle(isSuccess: boolean, isError: boolean, isPressed = false, isAdHoc = false): ViewStyle {
if (isSuccess) {
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
Expand All @@ -459,7 +459,7 @@ function getBadgeColorStyle(success: boolean, error: boolean, isPressed = false,
// eslint-disable-next-line @typescript-eslint/no-unsafe-return
return isPressed ? styles.badgeSuccessPressed : styles.badgeSuccess;
}
if (error) {
if (isError) {
// 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;
Expand Down Expand Up @@ -516,7 +516,7 @@ function getAnimatedFABStyle(rotate: Animated.Value, backgroundColor: Animated.V
};
}

function getWidthAndHeightStyle(width: number, height: number | undefined = undefined): ViewStyle {
function getWidthAndHeightStyle(width: number, height?: number): ViewStyle {
return {
width,
height: height ?? width,
Expand Down Expand Up @@ -909,12 +909,12 @@ function getReportWelcomeContainerStyle(isSmallScreenWidth: boolean): ViewStyle
/**
* Gets styles for AutoCompleteSuggestion row
*/
function getAutoCompleteSuggestionItemStyle(highlightedEmojiIndex: number, rowHeight: number, hovered: boolean, currentEmojiIndex: number): ViewStyle[] {
function getAutoCompleteSuggestionItemStyle(highlightedEmojiIndex: number, rowHeight: number, isHovered: boolean, currentEmojiIndex: number): ViewStyle[] {
let backgroundColor;

if (currentEmojiIndex === highlightedEmojiIndex) {
backgroundColor = themeColors.activeComponentBG;
} else if (hovered) {
} else if (isHovered) {
backgroundColor = themeColors.hoverComponentBG;
}

Expand Down Expand Up @@ -1023,7 +1023,7 @@ function getEmojiReactionCounterTextStyle(hasUserReacted: boolean): TextStyle {
*
* @param direction - The direction of the rotation (CONST.DIRECTION.LEFT or CONST.DIRECTION.RIGHT).
*/
function getDirectionStyle(direction: string): ViewStyle {
function getDirectionStyle(direction: ValueOf<typeof CONST.DIRECTION>): ViewStyle {
if (direction === CONST.DIRECTION.LEFT) {
return {transform: [{rotate: '180deg'}]};
}
Expand Down
4 changes: 2 additions & 2 deletions src/styles/addOutlineWidth/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ import AddOutlineWidth from './types';
/**
* Adds the addOutlineWidth property to an object to be used when styling
*/
const addOutlineWidth: AddOutlineWidth = (obj, val, error = false) => ({
const addOutlineWidth: AddOutlineWidth = (obj, val, hasError = false) => ({
...obj,
outlineWidth: val,
outlineStyle: val ? 'auto' : 'none',
boxShadow: val !== 0 ? `0px 0px 0px ${val}px ${error ? themeDefault.danger : themeDefault.borderFocus}` : 'none',
boxShadow: val !== 0 ? `0px 0px 0px ${val}px ${hasError ? themeDefault.danger : themeDefault.borderFocus}` : 'none',
});

export default addOutlineWidth;
2 changes: 1 addition & 1 deletion src/styles/addOutlineWidth/types.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
import {TextStyle} from 'react-native';

type AddOutlineWidth = (obj: TextStyle, val?: number, error?: boolean) => TextStyle;
type AddOutlineWidth = (obj: TextStyle, val?: number, hasError?: boolean) => TextStyle;

export default AddOutlineWidth;
2 changes: 1 addition & 1 deletion src/styles/getReportActionContextMenuStyles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ const miniWrapperStyle: ViewStyle[] = [
borderWidth: 1,
borderColor: themeColors.border,
// In Safari, when welcome messages use a code block (triple backticks), they would overlap the context menu below when there is no scrollbar without the transform style.
// NOTE: asserting "transform" to a valid type, because isn't possible to augment "transform".
// NOTE: asserting "transform" to a valid type, because it isn't possible to augment "transform".
transform: 'translateZ(0)' as unknown as ViewStyle['transform'],
},
];
Expand Down
8 changes: 4 additions & 4 deletions src/styles/utilities/display.ts
Original file line number Diff line number Diff line change
Expand Up @@ -23,31 +23,31 @@ export default {
* Web-only style.
*/
dInline: {
// NOTE: asserting "display" to a valid type, because isn't possible to augment "display".
// NOTE: asserting "display" to a valid type, because it isn't possible to augment "display".
display: 'inline' as ViewStyle['display'],
},

/**
* Web-only style.
*/
dInlineFlex: {
// NOTE: asserting "display" to a valid type, because isn't possible to augment "display".
// NOTE: asserting "display" to a valid type, because it isn't possible to augment "display".
display: 'inline-flex' as ViewStyle['display'],
},

/**
* Web-only style.
*/
dBlock: {
// NOTE: asserting "display" to a valid type, because isn't possible to augment "display".
// NOTE: asserting "display" to a valid type, because it isn't possible to augment "display".
display: 'block' as ViewStyle['display'],
},

/**
* Web-only style.
*/
dGrid: {
// NOTE: asserting "display" to a valid type, because isn't possible to augment "display".
// NOTE: asserting "display" to a valid type, because it isn't possible to augment "display".
display: 'grid' as ViewStyle['display'],
},
} satisfies Record<string, ViewStyle>;
2 changes: 1 addition & 1 deletion src/styles/utilities/overflowAuto/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import OverflowAutoStyles from './types';
* Web-only style.
*/
const overflowAuto: OverflowAutoStyles = {
// NOTE: asserting "overflow" to a valid type, because isn't possible to augment "overflow".
// NOTE: asserting "overflow" to a valid type, because it isn't possible to augment "overflow".
overflow: 'auto' as ViewStyle['overflow'],
};

Expand Down
2 changes: 1 addition & 1 deletion src/styles/utilities/positioning.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export default {
* Web-only style.
*/
pFixed: {
// NOTE: asserting "position" to a valid type, because isn't possible to augment "position".
// NOTE: asserting "position" to a valid type, because it isn't possible to augment "position".
position: 'fixed' as ViewStyle['position'],
},

Expand Down

0 comments on commit 05a58d8

Please sign in to comment.