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

Display columns on Record Board #3626

Merged
merged 5 commits into from
Jan 25, 2024

Conversation

charlesBochet
Copy link
Member

@charlesBochet charlesBochet commented Jan 25, 2024

Context

We are refactoring the record board which has been hard coded in the codebase to display opportunities.

What

In this PR:

  • adding state select field type on opportunity standard object
  • Introducing states to store columns: columnIds, columnFamilyState, columnFamilySelector (to ease usage), isColumnFirstFamilyState, isColumnLastFamilyState. We are introducing these familyState to guarantee good performance. A column should be able to read its columnInformation but do not re-render when other columns are changed.
  • using this state to compute board columns based on select field options (the first we find for now)
  • Re-adding columnHeader, columnMenu components but only redirecting to Settings to edit Select options

Next steps:

  • Load opportunities by column and display them through basic cards
  • re-apply filters and sort
  • re-apply viewFields
  • pagination (performance)
  • action bar, context menu, selection

@charlesBochet charlesBochet changed the title Display columns on available select field Display columns on Record Board Jan 25, 2024
return (
<RecordBoardScope
recordBoardScopeId={recordBoardId}
recordBoardScopeId={getScopeIdFromComponentIdStrict(recordBoardId)}
Copy link
Member

Choose a reason for hiding this comment

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

Why strict here? I feel like strict does not really tell what you want?

const setRecordBoardColumns = useRecoilCallback(
({ set, snapshot }) =>
(columns: RecordBoardColumnDefinition[]) => {
const currentColumns = snapshot
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const currentColumns = snapshot
const currentColumnIds = snapshot

Copy link
Contributor

@thomtrp thomtrp left a comment

Choose a reason for hiding this comment

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

LGTM, minor comments

columnDefinition: BoardColumnDefinition;
columnDefinition: RecordBoardColumnDefinition;
isColumnFirst: boolean;
isColumnLast: boolean;
Copy link
Contributor

Choose a reason for hiding this comment

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

I would have name is isFirst/LastColumn

@@ -0,0 +1,7 @@
import { createFamilyStateScopeMap } from '@/ui/utilities/recoil-scope/utils/createFamilyStateScopeMap';

export const isRecordBoardColumnFirstFamilyStateScopeMap =
Copy link
Contributor

Choose a reason for hiding this comment

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

I would also change this naming

const navigate = useNavigate();

const navigateToSelectSettings = useCallback(() => {
navigate(`/settings/objects/${objectNamePlural}`);
Copy link
Contributor

Choose a reason for hiding this comment

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

To store in AppPath?

Copy link
Member

@Weiko Weiko left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link

github-actions bot commented Jan 25, 2024

TODOs/FIXMEs:

  • // TODO: improve typing: packages/twenty-front/src/modules/ui/utilities/pointer-event/hooks/useClickOustideListenerStates.ts

Generated by 🚫 dangerJS against 5f3d97a

@charlesBochet charlesBochet merged commit 377fd23 into main Jan 25, 2024
13 checks passed
@charlesBochet charlesBochet deleted the display-columns-on-available-select-field branch January 25, 2024 17:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants