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

feat(web): translation management variants preview #5202

Merged
merged 43 commits into from
Feb 25, 2024

Conversation

LetItRock
Copy link
Contributor

What change does this PR introduce?

Implemented the new Variants Preview component.
All the design changes were approved by Nikolay and the difference is:

  • navigation when the variants have errors
  • we will not implement actions above the preview
  • preview positioning

Why was this change needed?

Other information (Screenshots)

Screenshot 2024-02-16 at 16 00 41

Screenshot 2024-02-16 at 16 00 30

Screenshot 2024-02-16 at 15 11 28
Screenshot 2024-02-16 at 15 11 15

Screen.Recording.2024-02-16.at.15.12.19.mov
Screen.Recording.2024-02-16.at.15.10.39.mov
Screen.Recording.2024-02-16.at.15.10.02.mov
Screen.Recording.2024-02-16.at.15.09.29.mov

@LetItRock LetItRock self-assigned this Feb 16, 2024
Copy link

linear bot commented Feb 16, 2024

Comment on lines +26 to +32
cy.intercept('GET', '**/v1/environments').as('environments');
cy.intercept('GET', '**/v1/organizations').as('organizations');
cy.intercept('GET', '**/v1/environments/me').as('environments-me');
cy.intercept('GET', '**/v1/notification-groups').as('notification-groups');
cy.intercept('GET', '**/v1/changes/count').as('changes-count');
cy.intercept('GET', '**/v1/integrations/active').as('active-integrations');
cy.intercept('GET', '**/v1/users/me').as('me');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

it was failing to start the tests this way locally for me

@@ -314,7 +314,7 @@ describe('Workflow Editor - Variants', function () {
fillEditorContent(channel, true);
goBack();

cy.getByTestId('editor-sidebar-add-variant').should('be.visible').click();
cy.getByTestId('variant-sidebar-add-variant').should('be.visible').click();
Copy link
Contributor Author

Choose a reason for hiding this comment

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

extracted a separate sidebar for variants preview, and gave a different test ids

@@ -162,7 +162,7 @@ function App() {
<Route path="providers" element={<ProvidersPage />} />
<Route path=":channel/:stepUuid" element={<ChannelStepEditor />} />
<Route path=":channel/:stepUuid/preview" element={<ChannelPreview />} />
<Route path=":channel/:stepUuid/variants" element={<VariantsPage />} />
<Route path=":channel/:stepUuid/variants/:variantUuid/preview" element={<VariantsPage />} />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the new preview variants route


type ResultType = { content: string };

export const usePreviewChat = (options: UseMutationOptions<ResultType, IResponseError, PayloadType> = {}) => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

refactored a little the preview chat and extracted this to a separate hook

Comment on lines +17 to +31
const PreviewContainer = styled.div`
display: flex;
flex-wrap: nowrap;
height: 100%;
margin: 0;
margin-bottom: 1.5rem;
gap: 1.5rem;
`;

const JSONContainer = styled.div`
min-width: 28rem;
height: 100%;
background: ${({ theme }) => (theme.colorScheme === 'dark' ? colors.B17 : colors.B98)};
border-radius: 7;
`;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I also did a little refactoring here. Use custom styled components instead of the Mantine grid components as it had many unnecessary styles that were breaking the layout.

onErrorDown: () => void;
}

export const VariantsListErrors: React.FC<IVariantsListErrorsProps> = ({
Copy link
Contributor Author

Choose a reason for hiding this comment

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

this is the code that was there before but extracted into a separate component


const variantsCreatePath = '/variants/create';

export const VariantsListSidebarActions = () => {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

these are the actions shown in the variants preview header, extracted from the existing code

Comment on lines +37 to +48
useEffect(() => {
if (allErrors.errors.length > 0) {
const errorIndex = 0;
const firstError = allErrors.errors[errorIndex];
setErrorState({
...firstError,
errorIndex,
});
} else {
setErrorState(null);
}
}, [allErrors]);
Copy link
Contributor Author

Choose a reason for hiding this comment

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

the first variant error from all errors is set

Comment on lines +50 to +79
const handleErrorNavigation = (direction: 'up' | 'down') => {
if (!errorState || !hasErrors) {
return;
}

const { errorIndex } = errorState;
const nextErrorIndex = direction === 'up' ? errorIndex - 1 : errorIndex + 1;

if (nextErrorIndex < 0 || nextErrorIndex >= allErrors.errors.length) {
return;
}

const error = allErrors.errors[nextErrorIndex];

setErrorState({
variantIndex: error.variantIndex,
errorMessage: error.errorMessage,
itemType: error.itemType,
errorIndex: nextErrorIndex,
errorType: error.errorType,
});

const isRootError = error?.itemType === ItemTypeEnum.ROOT;
if (isRootError) {
navigateToVariantPreview({ variantUuid: stepUuid });
} else if (typeof error.variantIndex !== 'undefined') {
const variant = variants[error.variantIndex];
navigateToVariantPreview({ variantUuid: variant.uuid });
}
};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

navigation logic through the errors

Comment on lines +44 to +46
const isNotStepEditorPath = pathname !== stepEditorPath;
const isStepWithNoVariant = isStepExists && !isVariantExists;
if (isStepWithNoVariant && isNotStepEditorPath) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed the issue with double navigation, when clicking on the workflow node card

Comment on lines 26 to 32
if (!locale) return;

getChatPreview({
content: templateContent,
payload: processedVariables,
locale,
});
Copy link
Contributor

Choose a reason for hiding this comment

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

Missing the disabled prop and to parse the payload. Otherwise the variables in the editor will not show a value in the preview

@ainouzgali ainouzgali merged commit 91ee23b into translation-management-beta Feb 25, 2024
18 of 19 checks passed
@ainouzgali ainouzgali deleted the nv-3416-variants-preview branch February 25, 2024 14:03
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.

None yet

3 participants