From 88a1804e18e01f084b9df8543e9f6f110a5b6bd9 Mon Sep 17 00:00:00 2001 From: PKulkoRaccoonGang Date: Thu, 21 Sep 2023 16:22:10 +0300 Subject: [PATCH 1/3] fix: fixed rtl direction in ProgressBar --- src/ProgressBar/index.jsx | 21 ++++++++++----- src/ProgressBar/index.scss | 15 +++++++++-- src/ProgressBar/tests/ProgressBar.test.jsx | 15 ++++++++--- src/ProgressBar/tests/utils.test.js | 30 +++++++++++++++++----- src/ProgressBar/utils.js | 16 ++++++++---- 5 files changed, 75 insertions(+), 22 deletions(-) diff --git a/src/ProgressBar/index.jsx b/src/ProgressBar/index.jsx index 1fe9cc5850..aafb58053f 100644 --- a/src/ProgressBar/index.jsx +++ b/src/ProgressBar/index.jsx @@ -31,18 +31,27 @@ function ProgressBarAnnotated({ thresholdHint, ...props }) { + const [direction, setDirection] = React.useState('ltr'); const progressInfoRef = React.useRef(); const thresholdInfoRef = React.useRef(); + const progressAnnotatedRef = React.useRef(); const thresholdPercent = (threshold || 0) - (now || 0); const isProgressHintAfter = now < HINT_SWAP_PERCENT; const isThresholdHintAfter = threshold < HINT_SWAP_PERCENT; const progressColor = VARIANTS.includes(variant) ? variant : PROGRESS_DEFAULT_VARIANT; const thresholdColor = VARIANTS.includes(thresholdVariant) ? thresholdVariant : THRESHOLD_DEFAULT_VARIANT; + useEffect(() => { + if (progressAnnotatedRef.current) { + const pageDirection = window.getComputedStyle(progressAnnotatedRef.current).getPropertyValue('direction'); + setDirection(pageDirection); + } + }, []); + const positionAnnotations = useCallback(() => { - placeInfoAtZero(progressInfoRef, isProgressHintAfter, ANNOTATION_CLASS); - placeInfoAtZero(thresholdInfoRef, isThresholdHintAfter, ANNOTATION_CLASS); - }, [isProgressHintAfter, isThresholdHintAfter]); + placeInfoAtZero(progressInfoRef, direction, isProgressHintAfter, ANNOTATION_CLASS); + placeInfoAtZero(thresholdInfoRef, direction, isThresholdHintAfter, ANNOTATION_CLASS); + }, [direction, isProgressHintAfter, isThresholdHintAfter]); useEffect(() => { positionAnnotations(); @@ -61,11 +70,11 @@ function ProgressBarAnnotated({ ); return ( -
+
{!!label && (
{!isProgressHintAfter && getHint(progressHint)} @@ -96,7 +105,7 @@ function ProgressBarAnnotated({ {(!!threshold && !!thresholdLabel) && (
{!isThresholdHintAfter && getHint(thresholdHint)} diff --git a/src/ProgressBar/index.scss b/src/ProgressBar/index.scss index 5b3532d62f..c323002a91 100644 --- a/src/ProgressBar/index.scss +++ b/src/ProgressBar/index.scss @@ -13,6 +13,7 @@ width: 100%; position: relative; overflow: visible; + padding: 3.125rem 0; .progress { overflow: visible; @@ -58,6 +59,11 @@ height: $progress-threshold-circle; border-radius: calc($progress-threshold-circle / 2); z-index: 1; + + [dir="rtl"] & { + left: -(calc($progress-threshold-circle / 2)); + right: auto; + } } } } @@ -77,8 +83,13 @@ } .pgn__progress-info { - display: inline-block; - position: relative; + position: absolute; + display: flex; + align-items: baseline; + + &:first-child { + top: 0; + } } .pgn__progress-hint { diff --git a/src/ProgressBar/tests/ProgressBar.test.jsx b/src/ProgressBar/tests/ProgressBar.test.jsx index 8e372a517b..e6a7d79824 100644 --- a/src/ProgressBar/tests/ProgressBar.test.jsx +++ b/src/ProgressBar/tests/ProgressBar.test.jsx @@ -1,5 +1,6 @@ import React from 'react'; import { mount } from 'enzyme'; +import { render } from '@testing-library/react'; import renderer from 'react-test-renderer'; import ProgressBar, { ANNOTATION_CLASS } from '..'; @@ -47,9 +48,8 @@ describe('', () => { expect(tree).toMatchSnapshot(); }); it('renders info blocks with calculated margins', () => { - const useReferenceSpy = jest.spyOn(React, 'useRef').mockReturnValue(ref); - mount(); - expect(useReferenceSpy).toHaveBeenCalledTimes(2); + jest.spyOn(React, 'useRef').mockReturnValue(ref); + render(); expect(ref.current.style.marginLeft).not.toBeFalsy(); }); it('renders correct variant for progress bar and annotation', () => { @@ -85,5 +85,14 @@ describe('', () => { expect(wrapper.find('.pgn__progress-info').get(0).props.children[2]).toEqual(false); expect(wrapper.find('.pgn__progress-info').get(1).props.children[2]).toEqual(false); }); + it('should apply styles based on direction for threshold', () => { + window.getComputedStyle = jest.fn().mockReturnValue({ getPropertyValue: () => 'rtl' }); + const { container } = render(); + const progressInfo = container.querySelector('.pgn__progress-info'); + const computedStyles = window.getComputedStyle(progressInfo); + + expect(computedStyles.getPropertyValue('directory')).toBe('rtl'); + window.getComputedStyle.mockRestore(); + }); }); }); diff --git a/src/ProgressBar/tests/utils.test.js b/src/ProgressBar/tests/utils.test.js index 740fb3790d..e6443e29cb 100644 --- a/src/ProgressBar/tests/utils.test.js +++ b/src/ProgressBar/tests/utils.test.js @@ -49,24 +49,42 @@ describe('utils', () => { expect(actualMarginLeft).toEqual(expectedMarginLeft); }); - it('correctly calculates left margin when annotationOnly equals to true', () => { - placeInfoAtZero(ref, false); + it('correctly calculates left margin when annotationOnly equals to true and dir equal ltr', () => { + placeInfoAtZero(ref, 'ltr', false); const { children } = ref.current; - let marginLeft = 0.0; + let horizontalMargin = 0.0; for (let i = 0; i < children.length || 0; i++) { const elementParams = children[i].getBoundingClientRect(); if (children[i].className.includes(ANNOTATION_CLASS)) { - marginLeft += elementParams.width / 2; + horizontalMargin += elementParams.width / 2; } else { - marginLeft += elementParams.width; + horizontalMargin += elementParams.width; } } - const expectedMarginLeft = `${-marginLeft}px`; + const expectedMarginLeft = `${-horizontalMargin}px`; const actualMarginLeft = ref.current.style.marginLeft; expect(actualMarginLeft).toEqual(expectedMarginLeft); }); + it('correctly calculates right margin when annotationOnly equals to true and dir equal rtl', () => { + placeInfoAtZero(ref, 'rtl', false); + + const { children } = ref.current; + let horizontalMargin = 0.0; + for (let i = 0; i < children.length || 0; i++) { + const elementParams = children[i].getBoundingClientRect(); + if (children[i].className.includes(ANNOTATION_CLASS)) { + horizontalMargin += elementParams.width / 2; + } else { + horizontalMargin += elementParams.width; + } + } + const expectedHorizontalMargin = `${-horizontalMargin}px`; + const actualMarginRight = ref.current.style.marginRight; + + expect(actualMarginRight).toEqual(expectedHorizontalMargin); + }); it('returns false if reference is wrong', () => { const wrongRef1 = {}; const wrongRef2 = { current: {} }; diff --git a/src/ProgressBar/utils.js b/src/ProgressBar/utils.js index 4d5f602224..63bd5c1b7d 100644 --- a/src/ProgressBar/utils.js +++ b/src/ProgressBar/utils.js @@ -3,24 +3,30 @@ * that the annotation pointer indicates on zero of the ProgressBar. * * @param {object} ref reference to the info block + * @param {string} direction directing elements to pages * @param {boolean} annotationOnly ignores width of the hint * @param {string} annotationClass is used to identify the annotation element */ // eslint-disable-next-line import/prefer-default-export -export const placeInfoAtZero = (ref, annotationOnly = true, annotationClass = 'pgn__annotation') => { +export const placeInfoAtZero = ( + ref, + direction = 'ltr', + annotationOnly = true, + annotationClass = 'pgn__annotation', +) => { if (!ref.current || !ref.current.style) { return false; } const { children } = ref.current; - let marginLeft = 0.0; + let horizontalMargin = 0.0; for (let i = 0; i < children.length || 0; i++) { const elementParams = children[i].getBoundingClientRect(); if (children[i].className.includes(annotationClass)) { - marginLeft += elementParams.width / 2; + horizontalMargin += elementParams.width / 2; } else { - marginLeft += annotationOnly ? 0.0 : elementParams.width; + horizontalMargin += annotationOnly ? 0.0 : elementParams.width; } } // eslint-disable-next-line no-param-reassign - ref.current.style.marginLeft = `${-marginLeft}px`; + ref.current.style[direction === 'rtl' ? 'marginRight' : 'marginLeft'] = `${-horizontalMargin}px`; return true; }; From 1757e879f30a73d6b61906c983ddeb390e909b92 Mon Sep 17 00:00:00 2001 From: PKulkoRaccoonGang Date: Fri, 22 Sep 2023 15:12:07 +0300 Subject: [PATCH 2/3] fix: refactoring after review --- src/ProgressBar/index.jsx | 19 ++++++++++--------- src/ProgressBar/utils.js | 16 ++++++++++++++-- 2 files changed, 24 insertions(+), 11 deletions(-) diff --git a/src/ProgressBar/index.jsx b/src/ProgressBar/index.jsx index aafb58053f..276df819a1 100644 --- a/src/ProgressBar/index.jsx +++ b/src/ProgressBar/index.jsx @@ -3,7 +3,7 @@ import ProgressBarBase from 'react-bootstrap/ProgressBar'; import PropTypes from 'prop-types'; import classNames from 'classnames'; import Annotation from '../Annotation'; -import { placeInfoAtZero } from './utils'; +import { getOffsetStyles, placeInfoAtZero } from './utils'; export const ANNOTATION_CLASS = 'pgn__annotation'; const HINT_SWAP_PERCENT = 50; @@ -20,7 +20,7 @@ function ProgressBar(props) { return ; } -function ProgressBarAnnotated({ +const ProgressBarAnnotated = React.forwardRef(({ now, label, variant, @@ -30,11 +30,12 @@ function ProgressBarAnnotated({ progressHint, thresholdHint, ...props -}) { +}, ref) => { const [direction, setDirection] = React.useState('ltr'); const progressInfoRef = React.useRef(); const thresholdInfoRef = React.useRef(); const progressAnnotatedRef = React.useRef(); + const resolvedRef = ref || progressAnnotatedRef; const thresholdPercent = (threshold || 0) - (now || 0); const isProgressHintAfter = now < HINT_SWAP_PERCENT; const isThresholdHintAfter = threshold < HINT_SWAP_PERCENT; @@ -42,11 +43,11 @@ function ProgressBarAnnotated({ const thresholdColor = VARIANTS.includes(thresholdVariant) ? thresholdVariant : THRESHOLD_DEFAULT_VARIANT; useEffect(() => { - if (progressAnnotatedRef.current) { - const pageDirection = window.getComputedStyle(progressAnnotatedRef.current).getPropertyValue('direction'); + if (resolvedRef.current) { + const pageDirection = window.getComputedStyle(resolvedRef.current).getPropertyValue('direction'); setDirection(pageDirection); } - }, []); + }, [resolvedRef]); const positionAnnotations = useCallback(() => { placeInfoAtZero(progressInfoRef, direction, isProgressHintAfter, ANNOTATION_CLASS); @@ -74,7 +75,7 @@ function ProgressBarAnnotated({ {!!label && (
{!isProgressHintAfter && getHint(progressHint)} @@ -105,7 +106,7 @@ function ProgressBarAnnotated({ {(!!threshold && !!thresholdLabel) && (
{!isThresholdHintAfter && getHint(thresholdHint)} @@ -120,7 +121,7 @@ function ProgressBarAnnotated({ )}
); -} +}); ProgressBarAnnotated.propTypes = { /** Current value of progress. */ diff --git a/src/ProgressBar/utils.js b/src/ProgressBar/utils.js index 63bd5c1b7d..451c49e60f 100644 --- a/src/ProgressBar/utils.js +++ b/src/ProgressBar/utils.js @@ -3,11 +3,10 @@ * that the annotation pointer indicates on zero of the ProgressBar. * * @param {object} ref reference to the info block - * @param {string} direction directing elements to pages + * @param {string} direction direction of the page ("ltr" or "rtl") * @param {boolean} annotationOnly ignores width of the hint * @param {string} annotationClass is used to identify the annotation element */ -// eslint-disable-next-line import/prefer-default-export export const placeInfoAtZero = ( ref, direction = 'ltr', @@ -30,3 +29,16 @@ export const placeInfoAtZero = ( ref.current.style[direction === 'rtl' ? 'marginRight' : 'marginLeft'] = `${-horizontalMargin}px`; return true; }; + +/** + * Retrieves offset styles based on the page direction. + * + * @param {number} value - The offset value in percentages. + * @param {string} direction - The offset direction ('rtl' for rightward offset, 'ltr' for leftward offset). + * @returns {Object} An object containing offset styles with either the 'left' or 'right' property, + * depending on the direction. + */ +export const getOffsetStyles = ( + value, + direction, +) => (direction === 'rtl' ? { right: `${value}%` } : { left: `${value}%` }); From 20c9585708ced16d4285fe75923b219b62a24de3 Mon Sep 17 00:00:00 2001 From: PKulkoRaccoonGang Date: Mon, 25 Sep 2023 12:32:57 +0300 Subject: [PATCH 3/3] refactor: code refactoring --- src/ProgressBar/index.jsx | 19 +++++-------------- 1 file changed, 5 insertions(+), 14 deletions(-) diff --git a/src/ProgressBar/index.jsx b/src/ProgressBar/index.jsx index 276df819a1..606f06f032 100644 --- a/src/ProgressBar/index.jsx +++ b/src/ProgressBar/index.jsx @@ -20,7 +20,7 @@ function ProgressBar(props) { return ; } -const ProgressBarAnnotated = React.forwardRef(({ +function ProgressBarAnnotated({ now, label, variant, @@ -30,24 +30,15 @@ const ProgressBarAnnotated = React.forwardRef(({ progressHint, thresholdHint, ...props -}, ref) => { - const [direction, setDirection] = React.useState('ltr'); +}) { const progressInfoRef = React.useRef(); const thresholdInfoRef = React.useRef(); - const progressAnnotatedRef = React.useRef(); - const resolvedRef = ref || progressAnnotatedRef; const thresholdPercent = (threshold || 0) - (now || 0); const isProgressHintAfter = now < HINT_SWAP_PERCENT; const isThresholdHintAfter = threshold < HINT_SWAP_PERCENT; const progressColor = VARIANTS.includes(variant) ? variant : PROGRESS_DEFAULT_VARIANT; const thresholdColor = VARIANTS.includes(thresholdVariant) ? thresholdVariant : THRESHOLD_DEFAULT_VARIANT; - - useEffect(() => { - if (resolvedRef.current) { - const pageDirection = window.getComputedStyle(resolvedRef.current).getPropertyValue('direction'); - setDirection(pageDirection); - } - }, [resolvedRef]); + const direction = window.getComputedStyle(document.body).getPropertyValue('direction'); const positionAnnotations = useCallback(() => { placeInfoAtZero(progressInfoRef, direction, isProgressHintAfter, ANNOTATION_CLASS); @@ -71,7 +62,7 @@ const ProgressBarAnnotated = React.forwardRef(({ ); return ( -
+
{!!label && (
); -}); +} ProgressBarAnnotated.propTypes = { /** Current value of progress. */