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

fix rename/updatemode and input issues #2199

Merged
merged 5 commits into from
Aug 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
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
5 changes: 5 additions & 0 deletions .changeset/curly-ravens-prove.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tokens-studio/figma-plugin': patch
---

Fixes an issue where renaming a token or token group would cause `Apply to` to be changed
5 changes: 5 additions & 0 deletions .changeset/rare-cups-join.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tokens-studio/figma-plugin': patch
---

Fixes an issue where the choice of `Rename styles` was not remembered per session
5 changes: 5 additions & 0 deletions .changeset/young-kangaroos-brake.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@tokens-studio/figma-plugin': patch
---

Fixes an issue with token edit inputs being focused after a timeout
17 changes: 7 additions & 10 deletions src/app/components/ConfirmDialog.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -79,16 +79,13 @@ function ConfirmDialog() {
}, [chosen, inputValue, confirmState, onConfirm]);

React.useEffect(() => {
const timeoutId = setTimeout(() => {
if (confirmState.choices) setChosen(confirmState.choices.filter((c) => c.enabled).map((c) => c.key));
if (firstInput.current) {
firstInput.current.focus();
} else if (confirmButton.current) {
confirmButton.current.focus();
}
}, 50);
return () => clearTimeout(timeoutId);
}, [confirmState.show, confirmButton, confirmState.choices]);
if (confirmState.choices) setChosen(confirmState.choices.filter((c) => c.enabled).map((c) => c.key));
if (firstInput.current) {
firstInput.current.focus();
} else if (confirmButton.current) {
confirmButton.current.focus();
}
}, [confirmState.show, confirmButton, confirmState.choices, firstInput]);

return confirmState.show ? (
<Modal isOpen close={onCancel}>
Expand Down
19 changes: 11 additions & 8 deletions src/app/components/EditTokenForm.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
import { checkIfAlias, checkIfContainsAlias, getAliasValue } from '@/utils/alias';
import { ResolveTokenValuesResult } from '@/utils/tokenHelpers';
import {
activeTokenSetSelector, updateModeSelector, editTokenSelector, themesListSelector, tokensSelector,
activeTokenSetSelector, editTokenSelector, themesListSelector, tokensSelector,
} from '@/selectors';
import { TokenTypes } from '@/constants/TokenTypes';
import TypographyInput from './TypographyInput';
Expand All @@ -40,6 +40,9 @@ import { MultiSelectDropdown } from './MultiSelectDropdown';
import { tokenTypesToCreateVariable } from '@/constants/VariableTypes';
import { ModalOptions } from '@/constants/ModalOptions';

let lastUsedRenameOption: UpdateMode = UpdateMode.SELECTION;
let lastUsedRenameStyles = false;

type Props = {
resolvedTokens: ResolveTokenValuesResult[];
};
Expand All @@ -53,7 +56,6 @@ function EditTokenForm({ resolvedTokens }: Props) {
const tokens = useSelector(tokensSelector);
const editToken = useSelector(editTokenSelector);
const themes = useSelector(themesListSelector);
const updateMode = useSelector(updateModeSelector);
const [selectedTokenSets, setSelectedTokenSets] = React.useState<string[]>([activeTokenSet]);
const {
editSingleToken, createSingleToken, duplicateSingleToken, renameTokensAcrossSets,
Expand Down Expand Up @@ -341,23 +343,23 @@ function EditTokenForm({ resolvedTokens }: Props) {
...($extensions ? { $extensions } : {}),
});
}
// When users change token names references are still pointing to the old name, ask user to remap
// When users change token names the applied tokens on layers are still pointing to the old name, ask user to remap
if (oldName && oldName !== newName) {
track('Edit token', { renamed: true, type: internalEditToken.type });
const choices: Choice[] = [
{
key: UpdateMode.SELECTION, label: 'Selection', unique: true, enabled: UpdateMode.SELECTION === updateMode,
key: UpdateMode.SELECTION, label: 'Selection', unique: true, enabled: UpdateMode.SELECTION === lastUsedRenameOption,
},
{
key: UpdateMode.PAGE, label: 'Page', unique: true, enabled: UpdateMode.PAGE === updateMode,
key: UpdateMode.PAGE, label: 'Page', unique: true, enabled: UpdateMode.PAGE === lastUsedRenameOption,
},
{
key: UpdateMode.DOCUMENT, label: 'Document', unique: true, enabled: UpdateMode.DOCUMENT === updateMode,
key: UpdateMode.DOCUMENT, label: 'Document', unique: true, enabled: UpdateMode.DOCUMENT === lastUsedRenameOption,
},
];
if (themes.length > 0 && [TokenTypes.COLOR, TokenTypes.TYPOGRAPHY, TokenTypes.BOX_SHADOW].includes(internalEditToken.type)) {
choices.push({
key: StyleOptions.RENAME, label: 'Rename styles',
key: StyleOptions.RENAME, label: 'Rename styles', enabled: lastUsedRenameStyles,
});
}
if (themes.length > 0 && tokenTypesToCreateVariable.includes(internalEditToken.type)) {
Expand All @@ -384,13 +386,14 @@ function EditTokenForm({ resolvedTokens }: Props) {
if (confirmData && confirmData.result) {
if (confirmData.data.some((data: string) => [UpdateMode.DOCUMENT, UpdateMode.PAGE, UpdateMode.SELECTION].includes(data as UpdateMode))) {
remapToken(oldName, newName, confirmData.data[0]);
dispatch.settings.setUpdateMode(confirmData.data[0]);
lastUsedRenameOption = confirmData.data[0] as UpdateMode;
}
if (confirmData.data.includes(ModalOptions.RENAME_ACROSS_SETS)) {
renameTokensAcrossSets(oldName, newName, type, tokenSetsContainsSameToken);
}
if (confirmData.data.includes(StyleOptions.RENAME)) {
renameStylesFromTokens({ oldName, newName, parent: activeTokenSet });
lastUsedRenameStyles = true;
}
if (confirmData.data.includes(ModalOptions.RENAME_VARIABLE)) {
renameVariablesFromToken({ oldName, newName });
Expand Down
4 changes: 1 addition & 3 deletions src/app/components/Input.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -184,9 +184,7 @@ const Input = React.forwardRef<HTMLInputElement, Props>(({

React.useEffect(() => {
if (autofocus && htmlInputRef && htmlInputRef.current) {
setTimeout(() => {
htmlInputRef.current?.focus();
}, 50);
htmlInputRef.current.focus();
}
}, [autofocus, htmlInputRef]);

Expand Down
7 changes: 6 additions & 1 deletion src/app/components/TokenTooltip/TokenTooltip.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
import React from 'react';
import { useSelector } from 'react-redux';
import { SingleToken } from '@/types/tokens';
import Tooltip from '../Tooltip';
import { TokenTooltipContent } from './TokenTooltipContent';
import { showEditFormSelector } from '@/selectors';

type Props = {
token: SingleToken;
Expand All @@ -11,14 +13,17 @@ export const TokenTooltip: React.FC<Props> = ({
children,
token,
}) => {
// When we open the token edit form we don't want tooltips to show through, which is happening sometimes
const showEditForm = useSelector(showEditFormSelector);

if (!children || !React.isValidElement(children)) {
return null;
}
Comment on lines 19 to 21
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
if (!children || !React.isValidElement(children)) {
return null;
}
if (!children || !React.isValidElement(children) || showEditForm) {
return null;
}

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

same as further down, that would cause jumps in layout as the button would not be rendered anymore. we just dont want the tooltip to be renderd


return (
<Tooltip
side="bottom"
label={(
label={showEditForm ? '' : (
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
label={showEditForm ? '' : (
label={(

Copy link
Collaborator

Choose a reason for hiding this comment

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

wouldn't it be better to add the condition above and just return null when the editForm is visible ?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

that would cause the children to also not be rendered though (Tooltip wraps the token button)

Copy link
Collaborator

Choose a reason for hiding this comment

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

ah got it. then the best way would be something like if (showEditForm) return children

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I'll include that in a followup 👍

<TokenTooltipContent
token={token}
/>
Expand Down
10 changes: 6 additions & 4 deletions src/app/store/useTokens.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -49,6 +49,8 @@ type GetFormattedTokensOptions = {

type RemoveTokensByValueData = { property: Properties; nodes: NodeInfo[] }[];

let lastUsedRenameOption: UpdateMode = UpdateMode.SELECTION;

export type SyncOption = 'removeStyle' | 'renameStyle';
export type SyncVariableOption = 'removeVariable' | 'renameVariable';

Expand Down Expand Up @@ -198,13 +200,13 @@ export default function useTokens() {
description: 'This will change all layers that used the old token name. This could take a while.',
choices: [
{
key: UpdateMode.SELECTION, label: 'Selection', unique: true, enabled: UpdateMode.SELECTION === settings.updateMode,
key: UpdateMode.SELECTION, label: 'Selection', unique: true, enabled: UpdateMode.SELECTION === lastUsedRenameOption,
},
{
key: UpdateMode.PAGE, label: 'Page', unique: true, enabled: UpdateMode.PAGE === settings.updateMode,
key: UpdateMode.PAGE, label: 'Page', unique: true, enabled: UpdateMode.PAGE === lastUsedRenameOption,
},
{
key: UpdateMode.DOCUMENT, label: 'Document', unique: true, enabled: UpdateMode.DOCUMENT === settings.updateMode,
key: UpdateMode.DOCUMENT, label: 'Document', unique: true, enabled: UpdateMode.DOCUMENT === lastUsedRenameOption,
},
{
key: 'rename-variable-token-group', label: 'Rename variable',
Expand All @@ -214,7 +216,7 @@ export default function useTokens() {
if (confirmData && confirmData.result) {
if (Array.isArray(confirmData.data) && confirmData.data.some((data: string) => [UpdateMode.DOCUMENT, UpdateMode.PAGE, UpdateMode.SELECTION].includes(data as UpdateMode))) {
await handleBulkRemap(newGroupName, oldGroupName, confirmData.data[0]);
dispatch.settings.setUpdateMode(confirmData.data[0] as UpdateMode);
lastUsedRenameOption = confirmData.data[0] as UpdateMode;
}
if (confirmData.data.includes('rename-variable-token-group')) {
track('renameVariablesInTokenGroup', { newGroupName, oldGroupName });
Expand Down
Loading