Feature: responsive pill count in emergency scenario visualisation for mobile#424
Feature: responsive pill count in emergency scenario visualisation for mobile#424
Conversation
WalkthroughThe EmergencyScenarioVisualization.vue component replaces a hard-coded constant maxVisiblePills value with a computed property that dynamically returns either 2 or 3 visible pills based on container width. When the container width is below 380 pixels, the component displays 2 pills; otherwise, it displays 3. All related calculations including overflow indicators, positioning, and rendering conditions are updated to reference this dynamic computed value instead of the fixed threshold. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
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. Comment Tip CodeRabbit can scan for known vulnerabilities in your dependencies using OSV Scanner.OSV Scanner will automatically detect and report security vulnerabilities in your project's dependencies. No additional configuration is required. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue (1)
97-120: Observe the container itself instead of onlywindow.resize.Line 114 recalculates
containerWidthonly on viewport resizes, but this component is also used inside parent-controlled dialog/grid layouts. If that parent width changes without awindow.resize,maxVisiblePillscan stick on the wrong breakpoint. AResizeObserveronpillContainerwould make this truly container-driven.♻️ Proposed change
const pillContainer = ref<HTMLElement | null>(null); const containerWidth = ref(0); +let resizeObserver: ResizeObserver | null = null; const maxVisiblePills = computed(() => { if (containerWidth.value > 0 && containerWidth.value < 380) return 2; return 3; }); @@ onMounted(() => { nextTick(() => { updateContainerWidth(); - window.addEventListener('resize', updateContainerWidth); + if (pillContainer.value) { + resizeObserver = new ResizeObserver(() => updateContainerWidth()); + resizeObserver.observe(pillContainer.value); + } }); }); onBeforeUnmount(() => { - window.removeEventListener('resize', updateContainerWidth); + resizeObserver?.disconnect(); + resizeObserver = null; stopRandomCouncilInterval(); });Please verify the supported browser matrix before relying solely on
ResizeObserver; if needed, keep the currentwindow.resizelistener as a fallback.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue` around lines 97 - 120, The component only updates containerWidth on window.resize, so when parent/layout changes without a viewport resize maxVisiblePills can be wrong; attach a ResizeObserver to pillContainer (the ref) in onMounted to call updateContainerWidth whenever the element's size changes, keep the existing window 'resize' listener as a fallback, and in onBeforeUnmount disconnect the ResizeObserver and still remove the window listener; ensure updateContainerWidth reads pillContainer.value.clientWidth and call it once after mounting, and keep stopRandomCouncilInterval() in onBeforeUnmount as-is.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue`:
- Around line 123-127: The pillWidth computation divides by the visibility cap
and can produce negative values; change it to divide by the actual number of
rendered pills (use Math.min(maxVisiblePills.value, requiredKeyShares.value) or
the appropriate reactive that represents actual shown pills) instead of
maxVisiblePills.value, compute usableWidth as Math.max(containerWidth.value -
totalGap - 100, 0) to avoid negatives, then compute Math.floor(usableWidth /
numPills) and only apply the fallback (200) when the result is not a positive
finite number (e.g., if result <= 0 or !isFinite(result) then return 200).
Reference pillWidth, maxVisiblePills, requiredKeyShares, and containerWidth when
updating the code.
---
Nitpick comments:
In `@frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue`:
- Around line 97-120: The component only updates containerWidth on
window.resize, so when parent/layout changes without a viewport resize
maxVisiblePills can be wrong; attach a ResizeObserver to pillContainer (the ref)
in onMounted to call updateContainerWidth whenever the element's size changes,
keep the existing window 'resize' listener as a fallback, and in onBeforeUnmount
disconnect the ResizeObserver and still remove the window listener; ensure
updateContainerWidth reads pillContainer.value.clientWidth and call it once
after mounting, and keep stopRandomCouncilInterval() in onBeforeUnmount as-is.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: bbd9244e-1114-4161-896e-cc02b8c74a13
📒 Files selected for processing (1)
frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue
| const pillWidth = computed(() => { | ||
| const totalGap = (maxVisiblePills - 1) * 8; | ||
| const numPills = maxVisiblePills.value; | ||
| const totalGap = (numPills - 1) * 8; | ||
| const usableWidth = Math.max(containerWidth.value - totalGap, 0) - 100; | ||
| return Math.floor(usableWidth / maxVisiblePills) || 200; | ||
| return Math.floor(usableWidth / numPills) || 200; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
fd "EmergencyScenarioVisualization.vue" --type fRepository: cryptomator/hub
Length of output: 133
🏁 Script executed:
fd "EmergencyAccessDialog.vue" --type fRepository: cryptomator/hub
Length of output: 195
🏁 Script executed:
cat -n frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue | head -150Repository: cryptomator/hub
Length of output: 6442
🏁 Script executed:
cat -n frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue | sed -n '150,250p'Repository: cryptomator/hub
Length of output: 3670
🏁 Script executed:
cat -n frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue | sed -n '250,300p'Repository: cryptomator/hub
Length of output: 1271
🏁 Script executed:
rg "EmergencyScenarioVisualization" -A 3 -B 3 --type vueRepository: cryptomator/hub
Length of output: 86
🏁 Script executed:
rg "EmergencyScenarioVisualization" -A 3 -B 3Repository: cryptomator/hub
Length of output: 7225
🏁 Script executed:
cat -n frontend/src/components/emergencyaccess/GrantEmergencyAccessDialog.vue | grep -A 20 "requiredKeyShares"Repository: cryptomator/hub
Length of output: 6887
🏁 Script executed:
cat -n frontend/src/components/emergencyaccess/EmergencyAccessDialog.vue | sed -n '1,100p'Repository: cryptomator/hub
Length of output: 5717
🏁 Script executed:
cat -n frontend/src/components/emergencyaccess/EmergencyAccessDialog.vue | sed -n '400,550p'Repository: cryptomator/hub
Length of output: 6497
🏁 Script executed:
cat -n frontend/src/components/CreateVault.vue | grep -A 5 "EmergencyScenarioVisualization"Repository: cryptomator/hub
Length of output: 669
🏁 Script executed:
cat -n frontend/src/components/AdminSettingsEmergencyAccess.vue | grep -B 10 -A 10 "EmergencyScenarioVisualization"Repository: cryptomator/hub
Length of output: 1997
🏁 Script executed:
rg "requiredKeyShares" frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue -B 2 -A 2Repository: cryptomator/hub
Length of output: 1503
🏁 Script executed:
rg "defaultRequiredEmergencyKeyShares\|defaultMinMembers" frontend/src/components/emergencyaccess/EmergencyAccessDialog.vue -B 2 -A 2Repository: cryptomator/hub
Length of output: 41
🏁 Script executed:
rg "defaultRequiredEmergencyKeyShares" frontend/src/components/ -B 2 -A 2Repository: cryptomator/hub
Length of output: 18560
🏁 Script executed:
node -e "console.log(-34 || 200); console.log(Math.floor(-100 / 3) || 200);"Repository: cryptomator/hub
Length of output: 66
Divide by actual rendered pills, not the visibility cap.
Line 124 divides by maxVisiblePills instead of the number of pills that will actually be shown. When requiredKeyShares < maxVisiblePills, this causes avoidable text truncation. Additionally, line 127's fallback || 200 doesn't catch negative widths: when containerWidth === 0, the calculation produces a negative value, and negative numbers are truthy in JavaScript, so -34 || 200 evaluates to -34, not 200.
🐛 Proposed change
const pillWidth = computed(() => {
- const numPills = maxVisiblePills.value;
+ const numPills = Math.min(requiredKeyShares.value, maxVisiblePills.value);
+ if (numPills === 0) return 0;
const totalGap = (numPills - 1) * 8;
const usableWidth = Math.max(containerWidth.value - totalGap, 0) - 100;
- return Math.floor(usableWidth / numPills) || 200;
+ return Math.max(Math.floor(usableWidth / numPills), 0);
});📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const pillWidth = computed(() => { | |
| const totalGap = (maxVisiblePills - 1) * 8; | |
| const numPills = maxVisiblePills.value; | |
| const totalGap = (numPills - 1) * 8; | |
| const usableWidth = Math.max(containerWidth.value - totalGap, 0) - 100; | |
| return Math.floor(usableWidth / maxVisiblePills) || 200; | |
| return Math.floor(usableWidth / numPills) || 200; | |
| const pillWidth = computed(() => { | |
| const numPills = Math.min(requiredKeyShares.value, maxVisiblePills.value); | |
| if (numPills === 0) return 0; | |
| const totalGap = (numPills - 1) * 8; | |
| const usableWidth = Math.max(containerWidth.value - totalGap, 0) - 100; | |
| return Math.max(Math.floor(usableWidth / numPills), 0); | |
| }); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@frontend/src/components/emergencyaccess/EmergencyScenarioVisualization.vue`
around lines 123 - 127, The pillWidth computation divides by the visibility cap
and can produce negative values; change it to divide by the actual number of
rendered pills (use Math.min(maxVisiblePills.value, requiredKeyShares.value) or
the appropriate reactive that represents actual shown pills) instead of
maxVisiblePills.value, compute usableWidth as Math.max(containerWidth.value -
totalGap - 100, 0) to avoid negatives, then compute Math.floor(usableWidth /
numPills) and only apply the fallback (200) when the result is not a positive
finite number (e.g., if result <= 0 or !isFinite(result) then return 200).
Reference pillWidth, maxVisiblePills, requiredKeyShares, and containerWidth when
updating the code.
|
Please rebase on commit d40ea08, as I eliminated the |
2 participants
maxVisiblePillsis now a computed value based on container width (2 on mobile < 380px, 3 on desktop)Problem
On mobile viewports, the emergency scenario visualization tried to fit 3 pills into a narrow container (~320px), resulting in each pill being only ~68px wide.
User names were almost completely hidden due to truncation.
Solution
On containers narrower than 380px, only 2 pills are shown (~106px each), with the remaining council members indicated by the existing +N counter badge.
Desktop behavior (3 pills) is unchanged.
Screenshots
Before
After