Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Resolve bugs tied to record creations on table #4011

Merged
merged 3 commits into from
Feb 16, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import { useState } from 'react';
import styled from '@emotion/styled';
import { useRecoilCallback, useSetRecoilState } from 'recoil';
import { useRecoilCallback, useRecoilState, useSetRecoilState } from 'recoil';

import { useColumnDefinitionsFromFieldMetadata } from '@/object-metadata/hooks/useColumnDefinitionsFromFieldMetadata';
import { useObjectMetadataItem } from '@/object-metadata/hooks/useObjectMetadataItem';
Expand All @@ -16,6 +15,7 @@ import { recordIndexFieldDefinitionsState } from '@/object-record/record-index/s
import { recordIndexFiltersState } from '@/object-record/record-index/states/recordIndexFiltersState';
import { recordIndexIsCompactModeActiveState } from '@/object-record/record-index/states/recordIndexIsCompactModeActiveState';
import { recordIndexSortsState } from '@/object-record/record-index/states/recordIndexSortsState';
import { recordIndexViewTypeState } from '@/object-record/record-index/states/recordIndexViewTypeState';
import { useRecordTable } from '@/object-record/record-table/hooks/useRecordTable';
import { SpreadsheetImportProvider } from '@/spreadsheet-import/provider/components/SpreadsheetImportProvider';
import { ViewBar } from '@/views/components/ViewBar';
Expand Down Expand Up @@ -46,10 +46,9 @@ export const RecordIndexContainer = ({
recordIndexId,
objectNamePlural,
}: RecordIndexContainerProps) => {
const [recordIndexViewType, setRecordIndexViewType] = useState<
ViewType | undefined
>(undefined);

const [recordIndexViewType, setRecordIndexViewType] = useRecoilState(
recordIndexViewTypeState,
);
const { objectNameSingular } = useObjectNameSingularFromPlural({
objectNamePlural,
});
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { atom } from 'recoil';

import { ViewType } from '@/views/types/ViewType';

export const recordIndexViewTypeState = atom<ViewType | undefined>({
key: 'recordIndexViewTypeState',
default: undefined,
});
Original file line number Diff line number Diff line change
@@ -1,59 +1,19 @@
import { useContext } from 'react';
import { useRecoilCallback, useRecoilValue } from 'recoil';

import { FieldContext } from '@/object-record/record-field/contexts/FieldContext';
import { useRecordFieldInput } from '@/object-record/record-field/hooks/useRecordFieldInput';
import { EntityDeleteContext } from '@/object-record/record-table/contexts/EntityDeleteHookContext';
import { RecordTableCellContext } from '@/object-record/record-table/contexts/RecordTableCellContext';
import { useRecordTableStates } from '@/object-record/record-table/hooks/internal/useRecordTableStates';
import { useDragSelect } from '@/ui/utilities/drag-select/hooks/useDragSelect';
import { useSetHotkeyScope } from '@/ui/utilities/hotkey/hooks/useSetHotkeyScope';
import { getSnapshotValue } from '@/ui/utilities/recoil-scope/utils/getSnapshotValue';

import { useCloseCurrentTableCellInEditMode } from '../../hooks/internal/useCloseCurrentTableCellInEditMode';
import { TableHotkeyScope } from '../../types/TableHotkeyScope';

export const useCloseRecordTableCell = () => {
const { getTableRowIdsState } = useRecordTableStates();
const { columnIndex } = useContext(RecordTableCellContext);
const { entityId, fieldDefinition } = useContext(FieldContext);
const deleteOneRecord = useContext(EntityDeleteContext);

const setHotkeyScope = useSetHotkeyScope();
const { setDragSelectionStartEnabled } = useDragSelect();

const closeCurrentTableCellInEditMode = useCloseCurrentTableCellInEditMode();

const {
getDraftValueSelector: getFieldInputDraftValueSelector,
isDraftValueEmpty: isCurrentFieldInputValueEmpty,
} = useRecordFieldInput(
`${entityId}-${fieldDefinition?.metadata?.fieldName}`,
);

const currentFieldInputDraftValue = useRecoilValue(
getFieldInputDraftValueSelector(),
);

const isFirstColumnCell = columnIndex === 0;

const deleteRow = useRecoilCallback(({ snapshot }) => async () => {
const tableRowIds = getSnapshotValue(snapshot, getTableRowIdsState());

await deleteOneRecord(tableRowIds[0]);
});

const closeTableCell = async () => {
setDragSelectionStartEnabled(true);
closeCurrentTableCellInEditMode();
setHotkeyScope(TableHotkeyScope.TableSoftFocus);

if (
isFirstColumnCell &&
isCurrentFieldInputValueEmpty(currentFieldInputDraftValue)
) {
await deleteRow();
}
};

return {
Expand Down
29 changes: 5 additions & 24 deletions packages/twenty-front/src/pages/object-record/RecordIndexPage.tsx
Original file line number Diff line number Diff line change
@@ -1,19 +1,15 @@
import { useParams } from 'react-router-dom';
import styled from '@emotion/styled';

import { useObjectMetadataItemForSettings } from '@/object-metadata/hooks/useObjectMetadataItemForSettings';
import { useObjectNameSingularFromPlural } from '@/object-metadata/hooks/useObjectNameSingularFromPlural';
import { useCreateOneRecord } from '@/object-record/hooks/useCreateOneRecord';
import { RecordIndexContainer } from '@/object-record/record-index/components/RecordIndexContainer';
import { DEFAULT_CELL_SCOPE } from '@/object-record/record-table/record-table-cell/hooks/useOpenRecordTableCell';
import { useSelectedTableCellEditMode } from '@/object-record/record-table/record-table-cell/hooks/useSelectedTableCellEditMode';
import { useIcons } from '@/ui/display/icon/hooks/useIcons';
import { PageAddButton } from '@/ui/layout/page/PageAddButton';
import { PageBody } from '@/ui/layout/page/PageBody';
import { PageContainer } from '@/ui/layout/page/PageContainer';
import { PageHeader } from '@/ui/layout/page/PageHeader';
import { PageHotkeysEffect } from '@/ui/layout/page/PageHotkeysEffect';
import { useSetHotkeyScope } from '@/ui/utilities/hotkey/hooks/useSetHotkeyScope';
import { RecordIndexPageHeader } from '~/pages/object-record/RecordIndexPageHeader';

const StyledIndexContainer = styled.div`
display: flex;
Expand All @@ -28,44 +24,29 @@ export const RecordIndexPage = () => {
objectNamePlural,
});

const { findObjectMetadataItemByNamePlural } =
useObjectMetadataItemForSettings();

const { getIcon } = useIcons();
const Icon = getIcon(
findObjectMetadataItemByNamePlural(objectNamePlural)?.icon,
);

const { createOneRecord: createOneObject } = useCreateOneRecord({
objectNameSingular,
});

const recordIndexId = objectNamePlural ?? '';

const setHotkeyScope = useSetHotkeyScope();

const { setSelectedTableCellEditMode } = useSelectedTableCellEditMode({
scopeId: recordIndexId,
});

const handleAddButtonClick = async () => {
await createOneObject?.({});
await createOneObject?.({
position: 0,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm assuming there is already one element at position 0 when you load the page right? Will it still work? @charlesBochet

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It works but it's a hack. We will need to do a better job but this is fixing the current erratic component. I'll create a follow up ticket to properly implement the position that should be first / 2

Copy link
Contributor

@martmull martmull Feb 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

behaviour is weird here, when I click on create button, new record is set at position 0, then when i click outside, the new record is moved to the end of the table. For numerous records table it will be a pain I guess
image
image

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@martmull this is actually tied to another behavior:
we should not create records with null position (which is happening when we create new workspaces or seed workspaces).
For existing workspace, my plan is to update all existing one in DB for now
For workspace, prefill I have added it to the PR
For demo + seed dev, I did it last week

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ok then go

});

setSelectedTableCellEditMode(0, 0);
setHotkeyScope(DEFAULT_CELL_SCOPE.scope, DEFAULT_CELL_SCOPE.customScopes);
};

return (
<PageContainer>
<PageHeader
title={
objectNamePlural.charAt(0).toUpperCase() + objectNamePlural.slice(1)
}
Icon={Icon}
>
<PageHotkeysEffect onAddButtonClick={handleAddButtonClick} />
<PageAddButton onClick={handleAddButtonClick} />
</PageHeader>
<RecordIndexPageHeader createRecord={handleAddButtonClick} />
<PageBody>
<StyledIndexContainer>
<RecordIndexContainer
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
import { useParams } from 'react-router-dom';
import { useRecoilValue } from 'recoil';

import { useObjectMetadataItemForSettings } from '@/object-metadata/hooks/useObjectMetadataItemForSettings';
import { recordIndexViewTypeState } from '@/object-record/record-index/states/recordIndexViewTypeState';
import { useIcons } from '@/ui/display/icon/hooks/useIcons';
import { PageAddButton } from '@/ui/layout/page/PageAddButton';
import { PageHeader } from '@/ui/layout/page/PageHeader';
import { PageHotkeysEffect } from '@/ui/layout/page/PageHotkeysEffect';
import { ViewType } from '@/views/types/ViewType';

type RecordIndexPageHeaderProps = {
createRecord: () => void;
};

export const RecordIndexPageHeader = ({
createRecord,
}: RecordIndexPageHeaderProps) => {
const objectNamePlural = useParams().objectNamePlural ?? '';

const { findObjectMetadataItemByNamePlural } =
useObjectMetadataItemForSettings();

const { getIcon } = useIcons();
const Icon = getIcon(
findObjectMetadataItemByNamePlural(objectNamePlural)?.icon,
);

const recordIndexViewType = useRecoilValue(recordIndexViewTypeState);

return (
<PageHeader
title={
objectNamePlural.charAt(0).toUpperCase() + objectNamePlural.slice(1)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@charlesBochet there is a capitalize util function in packages/twenty-front/src/utils/string/capitalize.ts

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've split a file in two so I missed it, but I'll use capitalize, thank you for pointing it

}
Icon={Icon}
>
<PageHotkeysEffect onAddButtonClick={createRecord} />
{recordIndexViewType === ViewType.Table && (
<PageAddButton onClick={createRecord} />
)}
</PageHeader>
);
};
Loading