fix(dashboard): Replace Select with Combobox for inline role editing#6621
fix(dashboard): Replace Select with Combobox for inline role editing#6621ssiyad wants to merge 2 commits into
Conversation
- Replace FormControl[type='select'] with Combobox in TeamInviteDialog - Replace Select with Combobox in Team member/invitation list - Move column definitions to computed() to fix reactivity hole where cells never re-rendered when roles loaded asynchronously - Add roleOptions computed to normalize data format (same as dialog) - Two-stage guard: permission check first, then loading state (shows Badge until roles arrive, then auto-transitions to Combobox) - Add openOnFocus and openOnClick to match read-only select behavior
|
| Filename | Overview |
|---|---|
| dashboard/src/components/settings/Team.vue | Migrated inline Select to Combobox with a computed columns definition and a roleOptions computed; the reactive mechanism is slightly misrepresented but functionally correct in practice. |
| dashboard/src/components/settings/TeamInviteDialog.vue | Replaced FormControl select with Combobox; manual label element is missing a for/id association with the combobox input. |
| press/press/doctype/team/team.py | Added cross-team ownership check in send_invitation (security fix) and refactored _validate_role/_set_invitation_role to accept a pre-fetched all_roles list, avoiding redundant DB calls. |
Sequence Diagram
sequenceDiagram
participant U as User (Admin)
participant TV as Team.vue
participant CB as Combobox cell
participant API as Backend (team.py)
TV->>API: get_roles (auto)
TV->>API: get_members (auto)
API-->>TV: members.data (rows)
Note over CB: roleOptions = [] → Badge shown
API-->>TV: roles.data → roleOptions computed updates
Note over CB: roleOptions.value.length > 0 → Combobox shown
U->>CB: Select new role
CB->>API: update_invitation_role(account_request, role)
API->>API: get_roles() → _validate_role(role, all_roles)
API->>API: "check d.team == self.name"
API->>API: _set_invitation_role(d, role, all_roles)
API-->>TV: updated members list
TV->>CB: members.setData(data) → row re-renders
Prompt To Fix All With AI
Fix the following 2 code review issues. Work through them one at a time, proposing concise fixes.
---
### Issue 1 of 2
dashboard/src/components/settings/TeamInviteDialog.vue:68-77
The `<label>` element has no `for` attribute, so it is not programmatically associated with the `Combobox` input. Clicking the label won't focus the picker, and screen readers won't announce "Role" when the combobox is focused — this is a regression from `FormControl[type="select"]` which handled association automatically.
```suggestion
<div v-if="roleOptions.length > 0">
<label
for="invite-role-combobox"
class="block text-xs leading-4 text-ink-gray-5 mb-1"
>
Role
</label>
<Combobox
id="invite-role-combobox"
v-model="selectedRole"
:options="roleOptions"
open-on-focus
/>
</div>
```
### Issue 2 of 2
dashboard/src/components/settings/Team.vue:120-230
**`columns` computed has no reactive dependencies**
The `computed(() => [...])` factory only constructs the array and function literals — it never executes the component function bodies, so `roleOptions.value` is never read during the factory run. Vue therefore records zero dependencies, meaning `columns` will never recompute and the parent will never re-render when roles load asynchronously. The fix actually works via a different path: `roleOptions.value` is read inside the closures when ObjectList calls `column.component({ row })` within its own reactive render context, which IS tracked correctly. Consider touching `roleOptions.value` at the top of the factory (e.g. `const opts = roleOptions.value; return [...]`) to make the dependency explicit and self-documenting.
Reviews (3): Last reviewed commit: "fix(team): Misc fixes" | Re-trigger Greptile
Codecov Report❌ Patch coverage is
❌ Your patch status has failed because the patch coverage (11.11%) is below the target coverage (75.00%). You can increase the patch coverage or adjust the target coverage. Additional details and impacted files@@ Coverage Diff @@
## develop #6621 +/- ##
===========================================
+ Coverage 42.54% 50.17% +7.62%
===========================================
Files 984 990 +6
Lines 82492 82971 +479
Branches 523 523
===========================================
+ Hits 35099 41633 +6534
+ Misses 47361 41306 -6055
Partials 32 32
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Problem
The Role column in the team member list used the legacy
Selectcomponent for inline role editing, while the invite dialog already used the newerCombobox. Additionally, when theget_rolesAPI loaded asynchronously, the list cells never re-rendered because Vue couldn't track dependencies referenced inside closures (the column component functions).Changes
dashboard: TeamInviteDialog — Replaced
FormControl[type="select"]with<Combobox>withopen-on-focus.dashboard: Team.vue — Key changes:
SelectwithComboboxfor role editing in both member and invitation rowscomputed()to fix a reactivity hole: whenroles.dataloaded asynchronously, the cell component functions were never re-invoked, so the guard always showed a Badge. Nowcolumnsis reactive — whenroleOptionschanges, the parent re-renders, giving cells fresh component functions with actual role data.roleOptionscomputed to normalize API data to{label, value}format (matching the dialog)openOnFocusandopenOnClickto match read-only select behaviorTesting
Reload the Team settings page. While roles are loading, the Role column shows blue Badges. Once loaded, they should transition to Comboboxes. Non-admins see only Badges.