-
-
Notifications
You must be signed in to change notification settings - Fork 203
Implement selective import of Variable Collections and Modes #3380
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
base: main
Are you sure you want to change the base?
Changes from 6 commits
edc0608
d8c6e9e
6f7f92b
7865686
1439af2
c29fad1
618ada6
50a2e4b
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,185 @@ | ||
import React, { useState, useCallback, useEffect } from 'react'; | ||
import { useTranslation } from 'react-i18next'; | ||
import { Button, Checkbox, Stack, Heading } from '@tokens-studio/ui'; | ||
import Modal from './Modal'; | ||
import Box from './Box'; | ||
import { VariableCollectionInfo, SelectedCollections } from '@/types/VariableCollectionSelection'; | ||
|
||
type Props = { | ||
isOpen: boolean; | ||
onClose: () => void; | ||
onConfirm: (selectedCollections: SelectedCollections, options: { useDimensions: boolean; useRem: boolean }) => void; | ||
collections: VariableCollectionInfo[]; | ||
}; | ||
|
||
export default function ImportVariablesDialog({ isOpen, onClose, onConfirm, collections }: Props) { | ||
const { t } = useTranslation(); | ||
const [selectedCollections, setSelectedCollections] = useState<SelectedCollections>({}); | ||
const [useDimensions, setUseDimensions] = useState(false); | ||
const [useRem, setUseRem] = useState(false); | ||
|
||
// Log collections data for debugging | ||
console.log('Collections passed to dialog:', collections); | ||
|
||
// Initialize all collections as selected with all modes selected by default | ||
useEffect(() => { | ||
if (collections.length > 0) { | ||
const initialSelection: SelectedCollections = {}; | ||
collections.forEach((collection) => { | ||
initialSelection[collection.id] = { | ||
name: collection.name, | ||
selectedModes: collection.modes.map((mode) => mode.modeId), | ||
}; | ||
}); | ||
setSelectedCollections(initialSelection); | ||
console.log('Initial selection set:', initialSelection); | ||
} | ||
}, [collections]); | ||
|
||
const handleCollectionToggle = useCallback((collectionId: string, collectionName: string, modes: { modeId: string; name: string }[]) => { | ||
setSelectedCollections((prev) => { | ||
const isCurrentlySelected = prev[collectionId]; | ||
if (isCurrentlySelected) { | ||
// Remove collection | ||
const newSelection = { ...prev }; | ||
delete newSelection[collectionId]; | ||
return newSelection; | ||
} else { | ||
// Add collection with all modes selected | ||
return { | ||
...prev, | ||
[collectionId]: { | ||
name: collectionName, | ||
selectedModes: modes.map((mode) => mode.modeId), | ||
}, | ||
}; | ||
} | ||
}); | ||
}, []); | ||
|
||
const handleModeToggle = useCallback((collectionId: string, modeId: string) => { | ||
setSelectedCollections((prev) => { | ||
const collection = prev[collectionId]; | ||
if (!collection) return prev; | ||
|
||
const isCurrentlySelected = collection.selectedModes.includes(modeId); | ||
const newSelectedModes = isCurrentlySelected | ||
? collection.selectedModes.filter((id) => id !== modeId) | ||
: [...collection.selectedModes, modeId]; | ||
|
||
// If no modes are selected, remove the collection entirely | ||
if (newSelectedModes.length === 0) { | ||
const newSelection = { ...prev }; | ||
delete newSelection[collectionId]; | ||
return newSelection; | ||
} | ||
|
||
return { | ||
...prev, | ||
[collectionId]: { | ||
...collection, | ||
selectedModes: newSelectedModes, | ||
}, | ||
}; | ||
}); | ||
}, []); | ||
|
||
const handleConfirm = useCallback(() => { | ||
onConfirm(selectedCollections, { useDimensions, useRem }); | ||
}, [selectedCollections, useDimensions, useRem, onConfirm]); | ||
|
||
const hasSelections = Object.keys(selectedCollections).length > 0; | ||
|
||
return ( | ||
<Modal | ||
title="Import variables" | ||
showClose | ||
isOpen={isOpen} | ||
close={onClose} | ||
footer={( | ||
<Stack direction="row" gap={4} justify="between"> | ||
<Button variant="secondary" onClick={onClose}> | ||
Cancel | ||
</Button> | ||
<Button variant="primary" disabled={!hasSelections} onClick={handleConfirm}> | ||
Import | ||
</Button> | ||
</Stack> | ||
)} | ||
> | ||
<Stack direction="column" gap={4} css={{ padding: '$4' }}> | ||
<Box css={{ fontSize: '$small', color: '$fgMuted' }}> | ||
Select which variable collections and modes to import. Sets will be created for each selected mode. | ||
</Box> | ||
|
||
{/* Options */} | ||
<Stack direction="column" gap={2}> | ||
<Heading size="small">Options</Heading> | ||
<Checkbox | ||
checked={useDimensions} | ||
onCheckedChange={(checked) => setUseDimensions(checked === true)} | ||
id="useDimensions" | ||
> | ||
Convert numbers to dimensions | ||
</Checkbox> | ||
<Checkbox | ||
checked={useRem} | ||
onCheckedChange={(checked) => setUseRem(checked === true)} | ||
id="useRem" | ||
> | ||
Use rem for dimension values | ||
</Checkbox> | ||
</Stack> | ||
|
||
{/* Collections */} | ||
<Stack direction="column" gap={3}> | ||
<Heading size="small">Variable Collections</Heading> | ||
{collections.map((collection) => { | ||
const isCollectionSelected = !!selectedCollections[collection.id]; | ||
const selectedModes = selectedCollections[collection.id]?.selectedModes || []; | ||
const allModesSelected = isCollectionSelected && selectedModes.length === collection.modes.length; | ||
|
||
return ( | ||
<Box key={collection.id} css={{ borderLeft: '2px solid $borderMuted', paddingLeft: '$3' }}> | ||
<Stack direction="column" gap={2}> | ||
<Checkbox | ||
checked={isCollectionSelected} | ||
onCheckedChange={() => handleCollectionToggle(collection.id, collection.name, collection.modes)} | ||
id={`collection-${collection.id}`} | ||
> | ||
<Box css={{ color: '$fgDefault', fontWeight: 'bold', userSelect: 'none' }}> | ||
{collection.name || `Collection ${collection.id.slice(0, 8)}`} | ||
</Box> | ||
</Checkbox> | ||
|
||
{isCollectionSelected && ( | ||
<Stack direction="column" gap={1} css={{ marginLeft: '$4' }}> | ||
{collection.modes.map((mode) => ( | ||
<Checkbox | ||
key={mode.modeId} | ||
checked={selectedModes.includes(mode.modeId)} | ||
onCheckedChange={() => handleModeToggle(collection.id, mode.modeId)} | ||
id={`mode-${collection.id}-${mode.modeId}`} | ||
> | ||
<Box css={{ color: '$fgDefault', userSelect: 'none' }}> | ||
{mode.name || `Mode ${mode.modeId.slice(0, 8)}`} | ||
</Box> | ||
</Checkbox> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You cannot nest text inside a checkbox like that. The checkbox is really onkly the checkbox itself. we'd usually do something like
this affects not just this line but other changes in this PR that you made regarding Checkbox There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed checkbox implementation to use proper pattern with separate Checkbox and Label components instead of nesting text inside checkboxes. Applied this pattern to all checkboxes in the ImportVariablesDialog (collection checkboxes, mode checkboxes, and option checkboxes) in commit 618ada6. |
||
))} | ||
</Stack> | ||
)} | ||
</Stack> | ||
</Box> | ||
); | ||
})} | ||
</Stack> | ||
|
||
{!hasSelections && ( | ||
<Box css={{ padding: '$3', backgroundColor: '$dangerBg', color: '$dangerFg', borderRadius: '$small' }}> | ||
Please select at least one collection and mode to import. | ||
</Box> | ||
)} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. add a special treatment: If there are no variable collections at all - render something that says "There are no collections present in this file" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added empty state handling with the message "There are no collections present in this file" when no variable collections are available in commit 618ada6. |
||
</Stack> | ||
</Modal> | ||
); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import { useState, useCallback } from 'react'; | ||
import { AsyncMessageChannel } from '@/AsyncMessageChannel'; | ||
import { AsyncMessageTypes } from '@/types/AsyncMessages'; | ||
import { VariableCollectionInfo, SelectedCollections } from '@/types/VariableCollectionSelection'; | ||
import { ThemeObjectsList } from '@/types/ThemeObjectsList'; | ||
|
||
export function useImportVariables() { | ||
const [isDialogOpen, setIsDialogOpen] = useState(false); | ||
const [collections, setCollections] = useState<VariableCollectionInfo[]>([]); | ||
const [isLoading, setIsLoading] = useState(false); | ||
|
||
const openDialog = useCallback(async () => { | ||
setIsLoading(true); | ||
try { | ||
// Fetch available collections first | ||
const response = await AsyncMessageChannel.ReactInstance.message({ | ||
type: AsyncMessageTypes.GET_AVAILABLE_VARIABLE_COLLECTIONS, | ||
}); | ||
|
||
console.log('Received collections in hook:', response.collections); | ||
setCollections(response.collections); | ||
setIsDialogOpen(true); | ||
} catch (error) { | ||
console.error('Error fetching variable collections:', error); | ||
setCollections([]); | ||
} finally { | ||
setIsLoading(false); | ||
} | ||
}, []); | ||
|
||
const closeDialog = useCallback(() => { | ||
setIsDialogOpen(false); | ||
setCollections([]); | ||
}, []); | ||
|
||
const importVariables = useCallback(async ( | ||
selectedCollections: SelectedCollections, | ||
options: { useDimensions: boolean; useRem: boolean }, | ||
themes: ThemeObjectsList, | ||
proUser: boolean | ||
) => { | ||
try { | ||
await AsyncMessageChannel.ReactInstance.message({ | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. let's add a track() analytics call here letting us know what options were chosen, and the amount of themes that people select here (number) as well as the total number of themes available There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added analytics tracking for variable imports with the following metrics: useDimensions, useRem options, number of selected collections vs total collections, and number of selected modes vs total available modes in commit 50a2e4b. |
||
type: AsyncMessageTypes.PULL_VARIABLES, | ||
options: { | ||
useDimensions: options.useDimensions, | ||
useRem: options.useRem, | ||
selectedCollections, | ||
}, | ||
themes, | ||
proUser, | ||
}); | ||
closeDialog(); | ||
} catch (error) { | ||
console.error('Error importing variables:', error); | ||
} | ||
}, [closeDialog]); | ||
|
||
return { | ||
isDialogOpen, | ||
collections, | ||
isLoading, | ||
openDialog, | ||
closeDialog, | ||
importVariables, | ||
}; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -190,28 +190,9 @@ export default function useTokens() { | |
}, [confirm]); | ||
|
||
const pullVariables = useCallback(async () => { | ||
const userDecision = await confirm({ | ||
text: 'Import variables', | ||
description: 'Sets will be created for each variable mode.', | ||
choices: [ | ||
{ key: 'useDimensions', label: 'Convert numbers to dimensions', enabled: false }, | ||
{ key: 'useRem', label: 'Use rem for dimension values', enabled: false }, | ||
], | ||
confirmAction: 'Import', | ||
}); | ||
|
||
if (userDecision) { | ||
AsyncMessageChannel.ReactInstance.message({ | ||
type: AsyncMessageTypes.PULL_VARIABLES, | ||
options: { | ||
useDimensions: userDecision.data.includes('useDimensions'), | ||
useRem: userDecision.data.includes('useRem'), | ||
}, | ||
themes, | ||
proUser, | ||
}); | ||
} | ||
}, [confirm, themes, proUser]); | ||
// This will be handled by the ImportVariablesDialog component | ||
// The dialog will be opened by the component using the useImportVariables hook | ||
}, []); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. given we no longer use this function, remove it entirely from this hook There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Removed the unused pullVariables function and its test in commits 3b9b6c6, ba8bb03, 18e4c59. |
||
|
||
const removeTokensByValue = useCallback((data: RemoveTokensByValueData) => { | ||
track('removeTokensByValue', { count: data.length }); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
remove all debugging logs that you added
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed all debugging console.log statements from ImportVariablesDialog and useImportVariables in commit 50a2e4b.