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

8414 add records selection context inside the command menu #8610

Merged

Conversation

bosiraphael
Copy link
Contributor

Closes #8414

Enregistrement.de.l.ecran.2024-11-20.a.16.27.44.mov

@Bonapara
Copy link
Member

Bonapara commented Nov 20, 2024

Will be awesome! Some feedbacks:

Dark Mode issue

CleanShot 2024-11-20 at 18 40 48

Need two escape to remove the selection

When focus is on "Type anything" first escape remove the focus on the input, second escape remove the selection

Removing the selection in breadcrumb should remove the selection on table/kanbans

If i press ⌫ or escape, should also deselect the record on the left

Header size

Should be 56px to match. Check all the header paddings

CleanShot 2024-11-20 at 18 47 48

Chip should be 32px heigh

put the border inside or make the chip 30px if the border are outside

CleanShot 2024-11-20 at 18 50 05

Delete doesn't work

CleanShot.2024-11-20.at.18.51.22.mp4

Icon identifiers

People icon identifiers should be rounded

CleanShot 2024-11-20 at 18 52 30

Tasks & Notes should use their colored icon image identifiers

CleanShot 2024-11-20 at 18 53 08

Empty state in wrongly aligned

CleanShot 2024-11-20 at 18 54 31

display: flex;
`;

const CommandMenuContextRecordChipAvatars = ({
Copy link
Member

Choose a reason for hiding this comment

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

1 type per file, 1 component per file

Copy link
Member

Choose a reason for hiding this comment

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

add stories

@@ -11,10 +11,10 @@ export const MainContextStoreComponentInstanceIdSetterEffect = () => {
const context = useContext(ContextStoreComponentInstanceContext);

useEffect(() => {
setMainContextStoreComponentInstanceId(context?.instanceId ?? null);
setMainContextStoreComponentInstanceId(context?.instanceId ?? 'app');
Copy link
Member

Choose a reason for hiding this comment

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

make a constant

export const useContextStoreCurrentObjectMetadataIdOrThrow = (
instanceId?: string,
) => {
const contextStoreCurrentObjectMetadataIdComponent =
Copy link
Member

Choose a reason for hiding this comment

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

naming

import { useFindManyRecords } from '@/object-record/hooks/useFindManyRecords';
import { useRecoilComponentValueV2 } from '@/ui/utilities/state/component-state/hooks/useRecoilComponentValueV2';

export const useContextStoreSelectedRecords = ({
Copy link
Member

Choose a reason for hiding this comment

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

useFindManyRecordSelectedInContextStore

@charlesBochet charlesBochet merged commit 8f5515c into main Nov 21, 2024
19 checks passed
@charlesBochet charlesBochet deleted the 8414-add-records-selection-context-inside-the-command-menu branch November 21, 2024 16:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add records selection context inside the command menu
3 participants