UX: Always show cancel and continue button in start/join room dialog#2333
Open
samuelwei wants to merge 4 commits into
Open
UX: Always show cancel and continue button in start/join room dialog#2333samuelwei wants to merge 4 commits into
samuelwei wants to merge 4 commits into
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## develop #2333 +/- ##
=============================================
+ Coverage 96.78% 97.20% +0.41%
=============================================
Files 468 183 -285
Lines 13364 6576 -6788
Branches 2215 2217 +2
=============================================
- Hits 12935 6392 -6543
+ Misses 429 184 -245 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
PILOS
 |
||||||||||||||||||||||||||||
| Project |
PILOS
|
| Branch Review |
ux-join-start-buttons-hidden
|
| Run status |
|
| Run duration | 07m 42s |
| Commit |
|
| Committer | Samuel Weirich |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
 |
0
|
 |
0
|
 |
0
|
 |
0
|
 |
632
|
| View all changes introduced in this branch ↗︎ | |
samuelwei
force-pushed
the
develop
branch
2 times, most recently
from
88ee280 to
bc9287a
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
samuelwei
force-pushed
the
ux-join-start-buttons-hidden
branch
from
d8bfba9 to
8328633
Compare
Collaborator
Author
|
@coderabbitai review |
Contributor
✅ Actions performedReview triggered.
|
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThis PR fixes small-screen layout issues in the room join dialog by introducing a scroll-aware wrapper component with dynamic shadow indicators, restructuring the form and footer into separate DOM regions with form-id-based submission wiring, refining content spacing, and documenting the fixes in the changelog. ChangesRoom join dialog layout and submission fix
Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes Suggested reviewers
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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 |
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
resources/js/components/RoomJoinButton.vue (1)
43-43:⚠️ Potential issue | 🟠 Major | ⚡ Quick winUse an instance-unique form id to prevent cross-dialog submit collisions.
Line 43 and Line 174 use a static
startJoinFormid/form target. With multipleRoomJoinButtoninstances mounted, this can submit the wrong form.Suggested fix
- <Form - id="startJoinForm" + <Form + :id="startJoinFormId" :disabled="isLoadingAction || loadingError" `@submit`="getJoinUrl" > ... - form="startJoinForm" + :form="startJoinFormId"const features = ref({}); +const startJoinFormId = computed(() => `startJoinForm-${props.roomId}`);Also applies to: 174-174
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@resources/js/components/RoomJoinButton.vue` at line 43, The static form id startJoinForm in RoomJoinButton causes cross-instance collisions; change the id usages to an instance-unique id by generating a per-component suffix (e.g. use the component's _uid or a small random/UUID string) and bind it: replace the literal "startJoinForm" on the form element and the target/form attributes (the template occurrences referenced in RoomJoinButton) with a computed/prop like startJoinFormId so each mounted instance has a distinct id; update any references in methods or attributes that target the form to use that bound startJoinFormId.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@resources/js/components/Dialog.vue`:
- Around line 33-35: The onModalHide function clears dialogContentRef but leaves
dialogContentScrollable unchanged, causing stale header/footer shadow state on
reopen; update onModalHide to also reset dialogContentScrollable (set
dialogContentScrollable.value = false) so scrollability and related shadow flags
are cleared when the modal hides.
---
Duplicate comments:
In `@resources/js/components/RoomJoinButton.vue`:
- Line 43: The static form id startJoinForm in RoomJoinButton causes
cross-instance collisions; change the id usages to an instance-unique id by
generating a per-component suffix (e.g. use the component's _uid or a small
random/UUID string) and bind it: replace the literal "startJoinForm" on the form
element and the target/form attributes (the template occurrences referenced in
RoomJoinButton) with a computed/prop like startJoinFormId so each mounted
instance has a distinct id; update any references in methods or attributes that
target the form to use that bound startJoinFormId.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: d328fbab-3c11-4fed-b07c-4a2226de4f05
📒 Files selected for processing (5)
CHANGELOG.mdresources/css/override/_dialog.cssresources/css/override/override.cssresources/js/components/Dialog.vueresources/js/components/RoomJoinButton.vue
✅ Files skipped from review due to trivial changes (1)
- resources/css/override/override.css
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Checklist
Changes
Other information
Initial UX issue when doing this change: On mobile no scrollbars are shown; same on mac os if a system setting is set. Users might not know that there is an addition checkbox and note if they are 'hidden' behind the buttons.
This issue is called Illusion of Completeness. This PR therefore adds the scroll-shadow design pattern to all dialogs, solving the Illusion of Completeness in most dialogs.
The following dialogs don't have a bottom content shadow due to a missing footer:
Summary by CodeRabbit
Bug Fixes
New Features
Style