Skip to content

Commit

Permalink
feat(MembersRoute): fix table overflow issues TASK-1349 (#5349)
Browse files Browse the repository at this point in the history
### 📣 Summary
Open `MemberRoleSelector` up and `MemberActionsDropdown` on the left -
to avoid both of them opening partially outside the container (and
causing an unnecessary scrollbar).

### 👀 Preview steps

1. ℹ️ have an organization with multiple members
2. log in as user having either "owner" or "admin" role
3. go to `#/account/organization/members`
4. 🔴 [on main] notice that `MemberRoleSelector` for last row opens
outside visible area causing scrollbar to happen
5. 🔴 [on main] notice that `MemberActionsDropdown` for last row opens
outside visible area causing scrollbar to happen
6. 🟢 [on PR] notice that both of the issues no longer happen, as both of
the components are opening in different placements
  • Loading branch information
magicznyleszek authored Dec 16, 2024
1 parent c1d68a4 commit 2c617e4
Show file tree
Hide file tree
Showing 5 changed files with 40 additions and 12 deletions.
7 changes: 4 additions & 3 deletions jsapp/js/account/organization/MemberActionsDropdown.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import {useState} from 'react';
import cx from 'classnames';

// Partial components
import KoboDropdown from 'jsapp/js/components/common/koboDropdown';
import KoboDropdown, {type KoboDropdownPlacement} from 'jsapp/js/components/common/koboDropdown';
import Button from 'jsapp/js/components/common/button';
import MemberRemoveModal from './MemberRemoveModal';

Expand All @@ -26,13 +26,14 @@ interface MemberActionsDropdownProps {
* wants to do the actions (not the role of the target member).
*/
currentUserRole: OrganizationUserRole;
placement: KoboDropdownPlacement;
}

/**
* A dropdown with all actions that can be taken towards an organization member.
*/
export default function MemberActionsDropdown(
{targetUsername, currentUserRole}: MemberActionsDropdownProps
{targetUsername, currentUserRole, placement}: MemberActionsDropdownProps
) {
const session = useSession();
const [isRemoveModalVisible, setIsRemoveModalVisible] = useState(false);
Expand Down Expand Up @@ -83,7 +84,7 @@ export default function MemberActionsDropdown(

<KoboDropdown
name={`member-actions-dropdown-${targetUsername}`}
placement='down-right'
placement={placement}
hideOnMenuClick
triggerContent={<Button type='text' size='m' startIcon='more'/>}
menuContent={
Expand Down
5 changes: 4 additions & 1 deletion jsapp/js/account/organization/MemberRoleSelector.tsx
Original file line number Diff line number Diff line change
@@ -1,17 +1,19 @@
import KoboSelect from 'jsapp/js/components/common/koboSelect';
import {usePatchOrganizationMember} from './membersQuery';
import {OrganizationUserRole} from './organizationQuery';
import {type KoboDropdownPlacement} from 'jsapp/js/components/common/koboDropdown';

interface MemberRoleSelectorProps {
username: string;
/** The role of the `username` user - the one we are modifying here. */
role: OrganizationUserRole;
/** The role of the currently logged in user. */
currentUserRole: OrganizationUserRole;
placement: KoboDropdownPlacement;
}

export default function MemberRoleSelector(
{username, role, currentUserRole}: MemberRoleSelectorProps
{username, role, currentUserRole, placement}: MemberRoleSelectorProps
) {
const patchMember = usePatchOrganizationMember(username);

Expand All @@ -25,6 +27,7 @@ export default function MemberRoleSelector(
name={`member-role-selector-${username}`}
type='outline'
size='m'
placement={placement}
options={[
{
value: OrganizationUserRole.admin,
Expand Down
16 changes: 14 additions & 2 deletions jsapp/js/account/organization/MembersRoute.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export default function MembersRoute() {
key: 'role',
label: t('Role'),
size: 120,
cellFormatter: (member: OrganizationMember) => {
cellFormatter: (member: OrganizationMember, rowIndex: number) => {
if (member.role === OrganizationUserRole.owner) {
return t('Owner');
}
Expand All @@ -78,6 +78,12 @@ export default function MembersRoute() {
username={member.user__username}
role={member.role}
currentUserRole={orgQuery.data.request_user_role}
// To avoid opening selector outside the container (causing
// unnecessary scrollbar), we open first 2 rows down, and the other
// rows up.
// TODO: this should be fixed by using a component with Portal
// functionality (looking at Mantine or MUI).
placement={rowIndex <= 1 ? 'down-center' : 'up-center'}
/>
);
},
Expand Down Expand Up @@ -105,7 +111,7 @@ export default function MembersRoute() {
label: '',
size: 64,
isPinned: 'right',
cellFormatter: (member: OrganizationMember) => {
cellFormatter: (member: OrganizationMember, rowIndex: number) => {
// There is no action that can be done on an owner
if (member.role === OrganizationUserRole.owner) {
return null;
Expand All @@ -115,6 +121,12 @@ export default function MembersRoute() {
<MemberActionsDropdown
targetUsername={member.user__username}
currentUserRole={orgQuery.data.request_user_role}
// To avoid opening selector outside the container (causing
// unnecessary scrollbar), we open first 2 rows down, and the other
// rows up.
// TODO: this should be fixed by using a component with Portal
// functionality (looking at Mantine or MUI).
placement={rowIndex <= 1 ? 'down-right' : 'up-right'}
/>
);
},
Expand Down
7 changes: 5 additions & 2 deletions jsapp/js/universalTable/universalTable.component.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ export interface UniversalTableColumn<DataItem> {
* the cell value. Without it a literal text value will be rendered. For more
* flexibility, function receives whole original data object.
*/
cellFormatter?: (value: DataItem) => React.ReactNode;
cellFormatter?: (value: DataItem, rowIndex: number) => React.ReactNode;
}

interface UniversalTableProps<DataItem> {
Expand Down Expand Up @@ -158,7 +158,10 @@ export default function UniversalTable<DataItem>(
header: () => columnDef.label,
cell: (cellProps: CellContext<DataItem, string>) => {
if (columnDef.cellFormatter) {
return columnDef.cellFormatter(cellProps.row.original);
return columnDef.cellFormatter(
cellProps.row.original,
cellProps.row.index
);
} else {
return cellProps.renderValue();
}
Expand Down
17 changes: 13 additions & 4 deletions jsapp/js/universalTable/universalTable.module.scss
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,11 @@ $z-index-resizer: 2;
// For sure pinned needs to be over .resizer, so it doesn't appear in
// weird/funny position when scrolling table horizontally.
$z-index-pinned: 3;
$z-index-pinned-header: 4;
$z-index-pinned-header-hover: 5;
$z-index-resizer-active: 6;
$z-index-spinner: 7;
$z-index-pinned-focused-cell: 4;
$z-index-pinned-header: 5;
$z-index-pinned-header-hover: 6;
$z-index-resizer-active: 7;
$z-index-spinner: 8;

.universalTableRoot {
border: 1px solid colors.$kobo-gray-200;
Expand Down Expand Up @@ -100,6 +101,14 @@ $z-index-spinner: 7;
z-index: $z-index-pinned;
}

// Needed so any elements inside of the cell (e.g. dropdown) don't open covered
// by background of cell below. This is IMPERFECT FIX, as when cell loses focus
// by clicking somewhere else - the background coverding blinks for split second.
.tableCell.isPinnedLeft:focus-within,
.tableCell.isPinnedRight:focus-within {
z-index: $z-index-pinned-focused-cell;
}

.tableHeaderCell.isPinnedLeft,
.tableHeaderCell.isPinnedRight, {
z-index: $z-index-pinned-header;
Expand Down

0 comments on commit 2c617e4

Please sign in to comment.