feat(12618): add tooltips for layers panel buttons on kontur apps #749
Conversation
WalkthroughThe recent updates enhance user interaction by integrating tooltips and a new 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: 6
Outside diff range and nitpick comments (3)
src/components/Layer/EraseControl/CleanControl.tsx (1)
Line range hint
18-22: Convert thecleanLayerfunction to an arrow function for consistency and conciseness.- const cleanLayer = useCallback( - async function () { - if (layerActions?.clean) { - layerActions.clean(); - } - }, - [layerActions], - ); + const cleanLayer = useCallback(async () => { + if (layerActions?.clean) { + layerActions.clean(); + } + }, [layerActions]);src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (2)
Line range hint
117-117: Avoid using non-null assertions. Consider handling potential undefined values gracefully.- const isGoodLeft = layer.sentiment[0] === 'good'; + const isGoodLeft = layer.sentiment?.[0] === 'good'; - label: layer.sentiment.at(0)!, + label: layer.sentiment?.at(0) ?? 'defaultLabel',Also applies to: 122-122
Line range hint
257-257: Avoid using array indices as keys in React components.- <div key={`nonDefault${index}`}>{`${v.paramName}: ${v.value}`}</div> + <div key={`nonDefault-${v.paramName}`}>{`${v.paramName}: ${v.value}`}</div>Use a unique identifier related to the item instead of the index.
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 (5)
- src/components/Layer/DownloadControl/DownloadControl.tsx (1 hunks)
- src/components/Layer/EditControl/EditControl.tsx (2 hunks)
- src/components/Layer/EraseControl/CleanControl.tsx (2 hunks)
- src/components/LayerHideControl/LayerHideControl.tsx (2 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (2 hunks)
Additional Context Used
Biome (13)
src/components/Layer/DownloadControl/DownloadControl.tsx (2)
6-6: Unexpected any. Specify a different type.
14-14: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
src/components/Layer/EditControl/EditControl.tsx (1)
30-30: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
src/components/Layer/EraseControl/CleanControl.tsx (2)
18-22: This function expression can be turned into an arrow function.
29-29: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
src/components/LayerHideControl/LayerHideControl.tsx (2)
21-21: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
33-33: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (6)
117-117: Forbidden non-null assertion.
122-122: Forbidden non-null assertion.
134-136: This else clause can be omitted because previous branches break early.
228-233: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
300-303: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
257-257: Avoid using the index of an array as key property in an element.
Additional comments not posted (5)
src/components/Layer/DownloadControl/DownloadControl.tsx (1)
2-3: Imports for tooltip functionality and localization have been added correctly.src/components/Layer/EraseControl/CleanControl.tsx (1)
3-4: Imports for tooltip functionality and localization have been added correctly.src/components/LayerHideControl/LayerHideControl.tsx (1)
2-3: Imports for tooltip functionality and localization have been added correctly.src/components/Layer/EditControl/EditControl.tsx (1)
3-5: Imports for tooltip functionality, feature flags, and localization have been added correctly.src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (1)
11-15: Imports for tooltip functionality have been added correctly.
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (1)
src/components/Layer/EraseControl/CleanControl.tsx (1)
Line range hint
17-21: Consider refactoring thecleanLayerfunction to use an arrow function for consistency and conciseness.- const cleanLayer = useCallback( - async function () { - if (layerActions?.clean) { - layerActions.clean(); - } - }, - [layerActions], - ); + const cleanLayer = useCallback(async () => { + if (layerActions?.clean) { + layerActions.clean(); + } + }, [layerActions]);
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (6)
- src/components/Layer/DownloadControl/DownloadControl.tsx (1 hunks)
- src/components/Layer/EditControl/EditControl.tsx (2 hunks)
- src/components/Layer/EraseControl/CleanControl.tsx (2 hunks)
- src/components/LayerActionButton/LayerActionButton.module.css (1 hunks)
- src/components/LayerActionButton/LayerActionIcon.tsx (1 hunks)
- src/components/LayerHideControl/LayerHideControl.tsx (2 hunks)
Files skipped from review as they are similar to previous changes (3)
- src/components/Layer/DownloadControl/DownloadControl.tsx
- src/components/Layer/EditControl/EditControl.tsx
- src/components/LayerHideControl/LayerHideControl.tsx
Additional context used
Biome
src/components/Layer/EraseControl/CleanControl.tsx
[error] 17-21: This function expression can be turned into an arrow function.
src/components/LayerActionButton/LayerActionIcon.tsx
[error] 38-38: The HTML element div is non-interactive. Do not use tabIndex.
Additional comments not posted (2)
src/components/LayerActionButton/LayerActionButton.module.css (1)
1-5: LGTM! The CSS changes are appropriate for enhancing the interactivity of the action buttons.src/components/LayerActionButton/LayerActionIcon.tsx (1)
1-46: LGTM! TheLayerActionIconcomponent is well-implemented with accessibility and tooltip functionality.Verification successful
The
tabIndexattribute is necessary for making thedivfocusable, which is crucial for keyboard accessibility. Removing it would hinder the ability to navigate using the keyboard.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify that the `tabIndex` is necessary for accessibility. # Test: Check if removing `tabIndex` affects accessibility. Expect: It should not be removed. echo "Ensure that tabIndex is necessary for keyboard accessibility and should not be removed despite the static analysis error."Length of output: 252
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (3)
src/components/Layer/EraseControl/CleanControl.tsx (1)
Line range hint
12-16: Consider refactoring the anonymous function to an arrow function for cleaner syntax.- async function () { + () => { if (layerActions?.clean) { layerActions.clean(); } },src/components/Layer/useControlElements.tsx (2)
Line range hint
33-33: Ensure all relevant dependencies are specified in theuseEffectdependency array to avoid potential bugs.- }, [layerState, layerActions, tooltipLayerId]); + }, [layerState, layerActions, tooltipLayerId, skipControls]);
Line range hint
38-38: Refactor string concatenation to use template literals for better readability and consistency.- key={layerState.id + 'hide'} + key={`${layerState.id}hide`} - key={layerState.id + 'edit'} + key={`${layerState.id}edit`} - key={layerState.id + 'clean'} + key={`${layerState.id}clean`} - key={layerState.id + 'download'} + key={`${layerState.id}download`} - key={layerState.id + 'context'} + key={`${layerState.id}context`}Also applies to: 49-49, 58-58, 68-68, 78-78
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/components/Layer/EraseControl/CleanControl.tsx (2 hunks)
- src/components/Layer/useControlElements.tsx (1 hunks)
Additional context used
Biome
src/components/Layer/EraseControl/CleanControl.tsx
[error] 12-16: This function expression can be turned into an arrow function.
src/components/Layer/useControlElements.tsx
[error] 38-38: Template literals are preferred over string concatenation.
[error] 49-49: Template literals are preferred over string concatenation.
[error] 58-58: Template literals are preferred over string concatenation.
[error] 68-68: Template literals are preferred over string concatenation.
[error] 78-78: Template literals are preferred over string concatenation.
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipContextMenu
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipLayerInfo
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipDownloadControl
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipVisibilityControl
Additional comments not posted (2)
src/components/Layer/EraseControl/CleanControl.tsx (2)
3-4: Imports for localization and new tooltip functionality are correctly added.
Line range hint
10-23: TheCleanControlcomponent correctly implements the tooltip usingLayerActionIconand ensures proper localization.
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (3)
Line range hint
113-113: Avoid using non-null assertions.Using non-null assertions (
!) can lead to runtime errors if the values are actually null. Consider adding null checks or default values.- layer.sentiment.at(0)! + layer.sentiment.at(0) ?? 'default_value'Also applies to: 118-118
Line range hint
130-132: This else clause can be omitted.The
elseclause is redundant here as the previous branches break early. Simplifying this can enhance code readability.- if (!isNumber(rangeFromNum)) { - errorFrom = i18n.t('mcda.layer_editor.errors.range_must_be_a_number'); - } - if (!isNumber(rangeToNum)) { - errorTo = i18n.t('mcda.layer_editor.errors.range_must_be_a_number'); - } - if (Number(rangeFrom) > Number(rangeTo)) { - errorFrom = i18n.t('mcda.layer_editor.errors.range_from_cannot_be_bigger'); - } - if (!rangeTo) { - errorTo = i18n.t('mcda.layer_editor.errors.range_cannot_be_empty'); - } - if (!rangeFrom) { - errorFrom = i18n.t('mcda.layer_editor.errors.range_cannot_be_empty'); - } + if (!isNumber(rangeFromNum) || !rangeFrom) { + errorFrom = i18n.t('mcda.layer_editor.errors.range_cannot_be_empty'); + } + if (!isNumber(rangeToNum) || !rangeTo) { + errorTo = i18n.t('mcda.layer_editor.errors.range_cannot_be_empty'); + } + if (Number(rangeFrom) > Number(rangeTo)) { + errorFrom = i18n.t('mcda.layer_editor.errors.range_from_cannot_be_bigger'); + }
Line range hint
251-251: Avoid using array index as key in React lists.Using array indices as keys in React can lead to issues with component state and re-rendering. Use unique, stable keys instead.
- <div key={`nonDefault${index}`}>{`${v.paramName}: ${v.value}`}</div> + <div key={`nonDefaultValue-${v.paramName}`}>{`${v.paramName}: ${v.value}`}</div>
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.module.css (1 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.module.css
Additional context used
Biome
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx
[error] 113-113: Forbidden non-null assertion.
[error] 118-118: Forbidden non-null assertion.
[error] 130-132: This else clause can be omitted because previous branches break early.
[error] 294-297: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 251-251: Avoid using the index of an array as key property in an element.
Additional comments not posted (1)
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (1)
11-11: Added import forLayerActionIconaligns with the PR's objective to enhance UI components with tooltips.
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range and nitpick comments (3)
src/components/Layer/EraseControl/CleanControl.tsx (1)
Line range hint
12-16: Consider converting the function expression to an arrow function for improved readability and consistency.- const cleanLayer = useCallback( - async function () { - if (layerActions?.clean) { - layerActions.clean(); - } - }, - [layerActions], - ); + const cleanLayer = useCallback(async () => { + if (layerActions?.clean) { + layerActions.clean(); + } + }, [layerActions]);src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (2)
Line range hint
113-113: Consider removing non-null assertions for better safety and simplifying the else clause to enhance code readability.- setSentiment(layer.sentiment.at(0) === 'good' ? 'good-bad' : 'bad-good'); + setSentiment(layer.sentiment[0] === 'good' ? 'good-bad' : 'bad-good'); - if (!rangeTo) { - errorTo = i18n.t('mcda.layer_editor.errors.range_cannot_be_empty'); - } + if (!rangeTo) errorTo = i18n.t('mcda.layer_editor.errors.range_cannot_be_empty');Also applies to: 118-118, 130-132
Line range hint
251-251: Use unique keys for list items to prevent potential issues with re-rendering. Also, ensure keyboard accessibility by addingonKeyUpevents.- <div key={`nonDefault${index}`}>{`${v.paramName}: ${v.value}`}</div> + <div key={`nonDefault-${v.paramName}`}>{`${v.paramName}: ${v.value}`}</div> - <div + <div onKeyUp={handleKeyUpReset}Also applies to: 294-297
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (7)
- src/components/Layer/DownloadControl/DownloadControl.tsx (1 hunks)
- src/components/Layer/EditControl/EditControl.tsx (2 hunks)
- src/components/Layer/EraseControl/CleanControl.tsx (2 hunks)
- src/components/LayerActionIcon/LayerActionIcon.module.css (1 hunks)
- src/components/LayerActionIcon/LayerActionIcon.tsx (1 hunks)
- src/components/LayerHideControl/LayerHideControl.tsx (2 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (2 hunks)
Files skipped from review due to trivial changes (1)
- src/components/LayerActionIcon/LayerActionIcon.module.css
Files skipped from review as they are similar to previous changes (2)
- src/components/Layer/DownloadControl/DownloadControl.tsx
- src/components/LayerHideControl/LayerHideControl.tsx
Additional context used
Biome
src/components/Layer/EraseControl/CleanControl.tsx
[error] 12-16: This function expression can be turned into an arrow function.
src/components/LayerActionIcon/LayerActionIcon.tsx
[error] 38-38: The HTML element div is non-interactive. Do not use tabIndex.
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx
[error] 113-113: Forbidden non-null assertion.
[error] 118-118: Forbidden non-null assertion.
[error] 130-132: This else clause can be omitted because previous branches break early.
[error] 294-297: Enforce to have the onClick mouse event with the onKeyUp, the onKeyDown, or the onKeyPress keyboard event.
[error] 251-251: Avoid using the index of an array as key property in an element.
Additional comments not posted (8)
src/components/Layer/EraseControl/CleanControl.tsx (2)
3-4: Imports and function signature changes are appropriate and align with the PR objectives.Also applies to: 10-10
21-23: The use ofLayerActionIconwith tooltips in JSX is correctly implemented and enhances user interaction.src/components/Layer/EditControl/EditControl.tsx (3)
3-6: Imports for localization andLayerActionIconare correctly added to enhance functionality.
21-27: The dynamic import within theeditLayerfunction is a good use of code splitting, enhancing performance by loading code only when needed.
30-32: The use ofLayerActionIconwith tooltips in JSX is correctly implemented and enhances user interaction.src/components/LayerActionIcon/LayerActionIcon.tsx (2)
1-11: Imports and interface definition inLayerActionIconare correctly implemented, supporting reusability and extensibility.
27-45: The implementation of tooltips and triggers in JSX is correctly done, enhancing accessibility and user interaction.Tools
Biome
[error] 38-38: The HTML element div is non-interactive. Do not use tabIndex.
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (1)
11-11: Import ofLayerActionIconis correctly added to enhance functionality.
…d-tooltips-for-layers-panel-buttons-on-kontur-apps
There was a problem hiding this comment.
Actionable comments posted: 1
Outside diff range comments (4)
src/components/Layer/EraseControl/CleanControl.tsx (1)
Line range hint
10-23: LGTM! The integration ofLayerActionIconwith localization support enhances the tooltip functionality.Consider converting the
cleanLayerfunction expression to an arrow function for consistency and potentially better performance in future React versions.- const cleanLayer = useCallback( - async function () { - if (layerActions?.clean) { - layerActions.clean(); - } - }, - [layerActions], - ); + const cleanLayer = useCallback(async () => { + if (layerActions?.clean) { + layerActions.clean(); + } + }, [layerActions]);src/components/Layer/useControlElements.tsx (1)
Line range hint
33-33: Ensure all dependencies are specified in theuseEffecthook to avoid potential bugs due to missing updates.- }, [layerState, layerActions, tooltipLayerId]); + }, [layerState, layerActions, tooltipLayerId, skipControls]);Tools
Biome
[error] 58-58: Template literals are preferred over string concatenation.
src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (2)
Line range hint
124-124: Avoid using non-null assertions 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)!, // Sentiments name needed instead of id - color: isGoodLeft ? sentimentColors.good : sentimentColors.bad, - value: String(layer.range.at(0)), - }, - right: { - label: layer.sentiment.at(1)!, - color: isGoodLeft ? sentimentColors.bad : sentimentColors.good, - value: String(layer.range.at(1)), - }, - }; + const isGoodLeft = layer.sentiment[0] === 'good'; + const leftLabel = layer.sentiment.at(0); + const rightLabel = layer.sentiment.at(1); + if (!leftLabel || !rightLabel) { + throw new Error('Sentiment labels are required.'); + } + return { + left: { + label: leftLabel, + color: isGoodLeft ? sentimentColors.good : sentimentColors.bad, + value: String(layer.range.at(0)), + }, + right: { + label: rightLabel, + color: isGoodLeft ? sentimentColors.bad : sentimentColors.good, + value: String(layer.range.at(1)), + }, + };Also applies to: 129-129
Line range hint
307-310: Ensure keyboard accessibility for all interactive elements to enhance usability for all users.+ onKeyUp={handleKeyUpEdit}
Review details
Configuration used: CodeRabbit UI
Review profile: ASSERTIVE
Files selected for processing (7)
- src/components/Layer/DownloadControl/DownloadControl.tsx (1 hunks)
- src/components/Layer/EditControl/EditControl.tsx (2 hunks)
- src/components/Layer/EraseControl/CleanControl.tsx (2 hunks)
- src/components/Layer/useControlElements.tsx (1 hunks)
- src/components/LayerHideControl/LayerHideControl.tsx (2 hunks)
- src/core/localization/translations/en/common.json (1 hunks)
- src/features/mcda/components/MCDALayerEditor/MCDALayerParameters/MCDALayerParameters.tsx (2 hunks)
Additional context used
Biome
src/components/Layer/EraseControl/CleanControl.tsx
[error] 12-16: This function expression can be turned into an arrow function.
src/components/Layer/useControlElements.tsx
[error] 38-38: Template literals are preferred over string concatenation.
[error] 49-49: Template literals are preferred over string concatenation.
[error] 58-58: Template literals are preferred over string concatenation.
[error] 68-68: Template literals are preferred over string concatenation.
[error] 78-78: Template literals are preferred over string concatenation.
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipContextMenu
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipLayerInfo
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipDownloadControl
[error] 33-33: This hook does not specify all of its dependencies: skipControls?.skipVisibilityControl
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] 307-310: 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 (4)
src/components/Layer/DownloadControl/DownloadControl.tsx (1)
5-12: LGTM! The integration ofLayerActionIconwith localization support enhances the tooltip functionality.src/components/LayerHideControl/LayerHideControl.tsx (1)
18-27: LGTM! The integration ofLayerActionIconwith localization support enhances the tooltip functionality for both hiding and showing layers.src/components/Layer/EditControl/EditControl.tsx (1)
30-32: LGTM! The integration ofLayerActionIconwith localization support enhances the tooltip functionality for the edit action.src/core/localization/translations/en/common.json (1)
34-42: The added tooltips for layer actions are correctly implemented and localized.
https://kontur.fibery.io/Tasks/My-tasks-3575#Task/Add-tooltips-for-Layers-panel-buttons-on-Kontur-apps-12618
Summary by CodeRabbit
New Features
LayerActionIconcomponent for consistent action icons in layer controls.Improvements
Localization