Custom screens : Sorting takes into account emoticones whereas it should not (#8239) #8467
Conversation
📝 WalkthroughWalkthroughA custom comparator function was added to the ag-grid configuration in the ✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 using PR comments)
Other keywords and placeholders
Documentation and Community
|
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
ui/main/src/app/views/customCardList/resultTable/ResultTable.ts (1)
40-40: Consider initializing comparator as undefined instead of empty object.Initializing
comparatoras an empty object{}seems unnecessary since it gets overwritten for STRING fields and remains unused for other field types. This could be confusing to maintainers.Consider this approach:
- comparator: {} + comparator: undefinedOr simply omit the property initialization and only set it when needed in the switch statement.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/main/src/app/views/customCardList/resultTable/ResultTable.ts(3 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
ui/main/src/app/views/customCardList/resultTable/ResultTable.ts (1)
ui/main/src/app/utils/Utilities.ts (1)
Utilities(14-114)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build
- GitHub Check: build
🔇 Additional comments (2)
ui/main/src/app/views/customCardList/resultTable/ResultTable.ts (2)
16-16: Import addition looks good.The import of
Utilitiesis correctly added and necessary for the new emoji-aware sorting functionality.
95-99: Emoji-aware comparator implementation is correct.The implementation properly addresses the PR objective by using
Utilities.compareObjwhich removes emojis before string comparison. The comparator function signature correctly matches AgGrid's expected interface.This change ensures that emoticons are excluded from sorting as intended, fixing the issue described in #8239.
b1169bc to
36c06b9
Compare
…uld not (#8239) Signed-off-by: vlo-rte <valerie.longa@rte-france.com>
36c06b9 to
fa6a6c0
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
ui/main/src/app/components/customCardList/CustomCardListComponent.ts(2 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
ui/main/src/app/components/customCardList/CustomCardListComponent.ts (1)
ui/main/src/app/utils/Utilities.ts (1)
Utilities(14-114)
🔇 Additional comments (1)
ui/main/src/app/components/customCardList/CustomCardListComponent.ts (1)
48-48: Import statement looks correct.The import of
Utilitiesfrom the relative path is properly added to support the emoji-aware sorting functionality.
| wrapText: false, | ||
| comparator: function (valueA: any, valueB: any) { | ||
| return Utilities.compareObj(valueA, valueB); | ||
| } |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Emoji-aware sorting successfully implemented for default columns.
The addition of the custom comparator using Utilities.compareObj correctly addresses the issue where emoticons were affecting sort order. This will ensure consistent sorting behavior by removing emojis before comparison.
However, verify that other column types with custom comparators don't also need emoji-aware sorting:
🏁 Script executed:
#!/bin/bash
# Description: Check if other column types might contain emoji-susceptible content
# Expected: Find column types that might benefit from emoji-aware sorting
echo "=== Checking column types with custom comparators that might need emoji handling ==="
echo
echo "1. TypeOfState comparator (lines 318-326):"
sed -n '318,326p' ui/main/src/app/components/customCardList/CustomCardListComponent.ts
echo -e "\n2. Responses comparator (lines 351-361):"
sed -n '351,361p' ui/main/src/app/components/customCardList/CustomCardListComponent.ts
echo -e "\n3. HTML comparator (lines 289-299):"
sed -n '289,299p' ui/main/src/app/components/customCardList/CustomCardListComponent.ts
echo -e "\n=== Searching for potential emoji usage in data sources ==="
rg -i "emoji|emoticon|🎯|⚠️" --type tsLength of output: 4290
Ensure emoji-aware sorting in all custom comparators
The default comparator now uses Utilities.compareObj, but the other custom comparators still perform direct string comparisons and will mis-order emoji-prefixed values. Please update the following to use Utilities.compareObj (or removeEmojis) on the relevant fields:
• ui/main/src/app/components/customCardList/CustomCardListComponent.ts
– Lines 289–299 (HTML comparator: replace if (rowValueA < rowValueB)… with return Utilities.compareObj(rowValueA, rowValueB))
– Lines 318–326 (TypeOfState comparator: replace text comparisons with return Utilities.compareObj(valueA.text, valueB.text))
– Lines 351–361 (Responses comparator: replace response string comparisons with return Utilities.compareObj(responseA, responseB))
By consolidating all comparators on Utilities.compareObj, we’ll guarantee consistent, emoji-agnostic sort order across every column.
🤖 Prompt for AI Agents
In ui/main/src/app/components/customCardList/CustomCardListComponent.ts at lines
289-299, 318-326, and 351-361, the custom comparators currently use direct
string comparisons which do not handle emoji-aware sorting. Replace the existing
comparison logic in these comparators with calls to Utilities.compareObj on the
relevant values (rowValueA and rowValueB for the HTML comparator, valueA.text
and valueB.text for the TypeOfState comparator, and responseA and responseB for
the Responses comparator) to ensure consistent emoji-agnostic sorting across all
columns.
|
Uh oh!
There was an error while loading. Please reload this page.