feat(18117): Add info popups for MCDA layer parameters #753
Conversation
|
WalkthroughThe updates enhance tooltip functionalities across various components by renaming Changes
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
Bundle size diff
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (1)
src/core/localization/translations/en/common.jsonis excluded by!**/*.json
Files selected for processing (2)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameterRow/MCDALayerParameterRow.tsx (1 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (6 hunks)
Additional context used
Biome
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx
[error] 112-112: Forbidden non-null assertion.
[error] 117-117: Forbidden non-null assertion.
[error] 129-131: This else clause can be omitted because previous branches break early.
[error] 221-226: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 293-296: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 247-247: Avoid using the index of an array as key property in an element.
Additional comments not posted (8)
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameterRow/MCDALayerParameterRow.tsx (3)
7-7: RenamingtipTexttoinfoTextaligns well with the intent to provide more descriptive and informative text.
13-13: The usage ofinfoTextin theTooltipTriggercomponent is correct and consistent with the interface changes.Also applies to: 22-22
13-24: TheMCDALayerParameterRowfunction is well-structured and effectively encapsulates the tooltip functionality.src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (5)
255-258: AddinginfoTextto theMCDALayerParameterRowfor the range parameter enhances user understanding of the parameter's purpose.
301-304: AddinginfoTextto theMCDALayerParameterRowfor the outliers parameter enhances user understanding of the parameter's purpose.
320-320: AddinginfoTextto theMCDALayerParameterRowfor the sentiment parameter enhances user understanding of the parameter's purpose.
335-338: AddinginfoTextto theMCDALayerParameterRowfor the weight parameter enhances user understanding of the parameter's purpose.
362-362: AddinginfoTextto theMCDALayerParameterRowfor the transform and normalize parameters enhances user understanding of the parameter's purpose.Also applies to: 379-379
albaranau
force-pushed
the
18117-add-info-popups-for-mcda-layer-parameters
branch
from
d813500
to
7ed4193
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (3)
- src/core/localization/translations/en/common.json (1 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameterRow/MCDALayerParameterRow.tsx (1 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (6 hunks)
Additional context used
Biome
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx
[error] 123-123: Forbidden non-null assertion.
[error] 128-128: Forbidden non-null assertion.
[error] 140-142: This else clause can be omitted because previous branches break early.
[error] 234-239: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 306-309: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 260-260: Avoid using the index of an array as key property in an element.
Additional comments not posted (4)
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameterRow/MCDALayerParameterRow.tsx (2)
7-7: RenametipTexttoinfoTextin theMCDALayerParameterRowPropsinterface.This change aligns with the PR's objective to standardize the naming convention for properties that provide additional information or tips.
13-13: Ensure consistent prop usage after renaming.The
tipTextprop has been correctly updated toinfoTextin the component's implementation. This change is consistent with the renaming in the interface.Also applies to: 22-22
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (1)
268-271: AddinfoTextproperties toMCDALayerParameterRowcomponents.The addition of
infoTextproperties to variousMCDALayerParameterRowcomponents enhances the user interface by providing informative text for parameters such as range, outliers, sentiment, weight, transform, and normalize. This aligns with the PR's objective to improve user comprehension and experience.Also applies to: 314-317, 333-333, 348-351, 375-375, 392-392
src/core/localization/translations/en/common.json (1)
122-129: Add detailed explanations for MCDA layer parameters in thetipssection.The addition of detailed explanations for various MCDA layer parameters such as range, sentiment, weight, transform, normalize, and outliers in the
tipssection of thecommon.jsonfile significantly enhances the clarity of information provided to users. This change supports the PR's goal of improving user guidance and understanding.
…d-info-popups-for-mcda-layer-parameters
…oid confusion with @konturio/floating
…id confusion with @konturio/floating
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range comments (10)
src/components/LayerInfo/LayerInfo.tsx (1)
Line range hint
45-47: Remove the unnecessary else clause to simplify the code.- } else { - return null; - }src/components/PopupTooltipTrigger/PopupTooltipTrigger.tsx (1)
Line range hint
15-55: Ensure accessibility by adding keyboard event handlers.+ function onKeyUp(e: React.KeyboardEvent<HTMLDivElement>) { + if (e.key === 'Enter') onClick(e as unknown as React.MouseEvent<HTMLDivElement, MouseEvent>); + } + return ( <div className={clsx(s.tooltip, className)} onClick={onClick} onPointerEnter={onPointerEnter} onPointerLeave={onPointerLeave} + onKeyUp={onKeyUp} tabIndex={0} // Ensure the div is focusable > {icon} </div> );src/components/PopupTooltipTrigger/PopupTooltipWrapper.tsx (1)
Line range hint
8-8: Specify a more precise type thananyfor better type safety.- type Args = any[]; + type Args = unknown[];src/features/bivariate_manager/components/BivariateMatrixControl/components/AxisCaptions/AxisCaptions.tsx (2)
Line range hint
44-44: Add keyboard accessibility to interactive elements.- <div className={s.tooltipHover} onClick={showTooltip}> + <div className={s.tooltipHover} onClick={showTooltip} onKeyUp={showTooltip} tabIndex="0">This change ensures that the tooltip can also be triggered via keyboard, enhancing accessibility.
Line range hint
61-68: Ensure SVG elements have appropriate alternative text for accessibility.- <svg xmlns="http://www.w3.org/2000/svg" width="6" height="22" viewBox="0 0 6 22" fill="none" transform={position === 'left' ? 'rotate(90)' : 'rotate(-90)'}> + <svg xmlns="http://www.w3.org/2000/svg" width="6" height="22" viewBox="0 0 6 22" fill="none" transform={position === 'left' ? 'rotate(90)' : 'rotate(-90)'} aria-label="Long Arrow">Adding
aria-labelprovides a textual description of the SVG's purpose, which is crucial for screen reader users.src/components/BivariateLegend/CornerTooltipWrapper.tsx (1)
Line range hint
70-70: Avoid using array indices as keys in React components to prevent potential issues with component state and re-rendering.- {rows.map(({ label, direction, indicator }, i) => ( - <div key={i} className={clsx(s.tooltipRow)}> + {rows.map(({ label, direction, indicator }, i) => ( + <div key={`${label}-${i}`} className={clsx(s.tooltipRow)}>Using a combination of the label and index as the key can help ensure that the keys are unique and stable.
src/features/bivariate_manager/components/BivariateMatrixControl/components/DenominatorIcon/DenominatorIcon.tsx (1)
Line range hint
10-16: Ensure SVG elements have appropriate alternative text for accessibility.- <svg width="24" height="24" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg"> + <svg width="24" height="24" viewBox="0 0 15 15" fill="none" xmlns="http://www.w3.org/2000/svg" aria-label="Icon Description">Adding
aria-labelprovides a textual description of the SVG's purpose, which is crucial for screen reader users.Also applies to: 40-46, 55-61, 72-78, 89-95, 106-112
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (3)
Line range hint
124-124: Avoid non-null assertions in TypeScript as they can lead to runtime errors if assumptions about non-nullability prove incorrect.- const isGoodLeft = layer.sentiment[0] === 'good'; - return { - left: { - label: layer.sentiment.at(0)!, + const isGoodLeft = layer.sentiment[0] === 'good'; + const leftLabel = layer.sentiment.at(0) || 'default'; + return { + left: { + label: leftLabel,This change ensures that the code handles potential null or undefined values gracefully.
Also applies to: 129-129
Line range hint
310-313: Add keyboard accessibility to interactive elements.- <div className={clsx(s.resetLimits, { [s.textButtonDisabled]: axes.loading })} onClick={onResetLimits}> + <div className={clsx(s.resetLimits, { [s.textButtonDisabled]: axes.loading })} onClick={onResetLimits} onKeyUp={onResetLimits} tabIndex="0">This change ensures that the reset limits button can also be triggered via keyboard, enhancing accessibility.
Line range hint
264-264: Avoid using array indices as keys in React components to prevent potential issues with component state and re-rendering.- {nonDefaultValues.map((v, index) => ( - <div key={`nonDefault${index}`}>{`${v.paramName}: ${v.value}`}</div> + {nonDefaultValues.map((v, index) => ( + <div key={`${v.paramName}-${index}`}>{`${v.paramName}: ${v.value}`}</div>Using a combination of the parameter name and index as the key can help ensure that the keys are unique and stable.
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (11)
- src/components/BivariateLegend/CornerTooltipWrapper.tsx (2 hunks)
- src/components/LabelWithTooltip/LabelWithTooltip.tsx (2 hunks)
- src/components/LayerInfo/LayerInfo.tsx (2 hunks)
- src/components/PopupTooltipTrigger/PopupTooltipTrigger.tsx (2 hunks)
- src/components/PopupTooltipTrigger/PopupTooltipWrapper.tsx (2 hunks)
- src/components/PopupTooltipTrigger/index.ts (1 hunks)
- src/core/localization/translations/en/common.json (1 hunks)
- src/features/bivariate_manager/components/BivariateMatrixControl/components/AxisCaptions/AxisCaptions.tsx (3 hunks)
- src/features/bivariate_manager/components/BivariateMatrixControl/components/DenominatorIcon/DenominatorIcon.tsx (3 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameterRow/MCDALayerParameterRow.tsx (1 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (6 hunks)
Additional context used
Biome
src/components/LayerInfo/LayerInfo.tsx
[error] 45-47: This else clause can be omitted because previous branches break early.
src/components/PopupTooltipTrigger/PopupTooltipTrigger.tsx
[error] 50-55: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
src/components/PopupTooltipTrigger/PopupTooltipWrapper.tsx
[error] 8-8: Unexpected any. Specify a different type.
src/features/bivariate_manager/components/BivariateMatrixControl/components/AxisCaptions/AxisCaptions.tsx
[error] 44-44: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 61-68: Alternative text title element cannot be empty
src/components/BivariateLegend/CornerTooltipWrapper.tsx
[error] 70-70: Avoid using the index of an array as key property in an element.
src/features/bivariate_manager/components/BivariateMatrixControl/components/DenominatorIcon/DenominatorIcon.tsx
[error] 10-16: Alternative text title element cannot be empty
[error] 40-46: Alternative text title element cannot be empty
[error] 55-61: Alternative text title element cannot be empty
[error] 72-78: Alternative text title element cannot be empty
[error] 89-95: Alternative text title element cannot be empty
[error] 106-112: Alternative text title element cannot be empty
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx
[error] 124-124: Forbidden non-null assertion.
[error] 129-129: Forbidden non-null assertion.
[error] 141-143: This else clause can be omitted because previous branches break early.
[error] 310-313: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 264-264: Avoid using the index of an array as key property in an element.
Additional comments not posted (20)
src/components/PopupTooltipTrigger/index.ts (2)
1-1: Export statement forPopupTooltipTriggeris correctly updated.
2-2: Export statement forPopupTooltipWrapperis correctly updated.src/components/LabelWithTooltip/LabelWithTooltip.tsx (2)
1-1: Import statement forPopupTooltipTriggeris correctly updated.
17-17: Usage ofPopupTooltipTriggerwithinLabelWithTooltipcomponent is correctly updated.src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameterRow/MCDALayerParameterRow.tsx (3)
1-1: Import statement forPopupTooltipTriggeris correctly updated.
7-7: Update to theMCDALayerParameterRowPropstype definition is correctly implemented.
20-24: Usage ofPopupTooltipTriggerwithinMCDALayerParameterRowcomponent is correctly updated.src/components/LayerInfo/LayerInfo.tsx (2)
2-2: Import statement forPopupTooltipTriggeris correctly updated.
39-39: Usage ofPopupTooltipTriggerwithinLayerInfocomponent is correctly updated.src/components/PopupTooltipTrigger/PopupTooltipTrigger.tsx (1)
5-5: Import statement for CSS module is correctly updated.src/components/PopupTooltipTrigger/PopupTooltipWrapper.tsx (2)
Line range hint
1-1: Import statement foruseActionis correctly updated.
Line range hint
29-77: Definition ofPopupTooltipWrapperis correctly implemented.src/features/bivariate_manager/components/BivariateMatrixControl/components/AxisCaptions/AxisCaptions.tsx (2)
4-4: Updated import to usePopupTooltipWrapperaligns with the renaming of tooltip components across the project.
Line range hint
33-54: ThePopupTooltipWrappercomponent is correctly implemented with the new tooltip text structure. Ensure that the tooltip content is accessible and provides meaningful information.src/components/BivariateLegend/CornerTooltipWrapper.tsx (2)
3-3: Updated import to usePopupTooltipWrapperaligns with the renaming of tooltip components across the project.
34-42: ThePopupTooltipWrappercomponent is correctly implemented with the new tooltip logic. Ensure that the tooltip content is accessible and provides meaningful information.src/features/bivariate_manager/components/BivariateMatrixControl/components/DenominatorIcon/DenominatorIcon.tsx (2)
2-2: Updated import to usePopupTooltipWrapperaligns with the renaming of tooltip components across the project.
Line range hint
143-153: ThePopupTooltipWrappercomponent is correctly implemented with the new tooltip logic. Ensure that the tooltip content is accessible and provides meaningful information.src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (1)
Line range hint
272-396: The addition ofinfoTexttoMCDALayerParameterRowcomponents enhances the user interface by providing more detailed information for MCDA layer parameters.src/core/localization/translations/en/common.json (1)
133-138: The added tooltips under the "tips" section are clear and informative, enhancing user understanding of the MCDA layer parameters.
https://kontur.fibery.io/Tasks/Task/Add-info-popups-for-MCDA-layer-parameters-18117
Summary by CodeRabbit
New Features
Refactor
tipTexttoinfoTextfor better clarity across multiple components.TooltipWrapperwithPopupTooltipWrapperto improve tooltip behavior and consistency across components.