Skip to content

Commit

Permalink
fix: update left nav and top bar strings (#7609)
Browse files Browse the repository at this point in the history
* start fixing header

* adjust header chrome and left nav order

* Update en-US.json

* fix typecheck errors

* fix unit tests

* Update en-US.json

* change Design to Create in e2e tests

* Update LuisDeploy.spec.ts

* update flaky check in visitPage

* add 'checked' flag to visitPage to minimize surface area

* fix punctuation

* fix e2e test!

* fix security issue

* Revert "fix security issue"

This reverts commit aa008c1.

Co-authored-by: Chris Whitten <christopher.whitten@microsoft.com>
  • Loading branch information
beyackle and cwhitten committed May 5, 2021
1 parent 5054312 commit c66c44d
Show file tree
Hide file tree
Showing 15 changed files with 76 additions and 94 deletions.
2 changes: 1 addition & 1 deletion Composer/cypress/integration/DesignPage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ context('breadcrumb', () => {
});

it('can create different kinds of triggers ', () => {
cy.visitPage('Design');
cy.visitPage('Create');
cy.findByTestId('DialogHeader-TestBot_TestSample').click();
cy.findByTestId('recognizerTypeDropdown').click();
cy.findByText('Regular expression recognizer').click();
Expand Down
2 changes: 1 addition & 1 deletion Composer/cypress/integration/HomePage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ context('Home Page ', () => {
cy.createTestBot('EmptySample', ({ id }) => {
cy.visit(`/bot/${id}`);
cy.findByTestId('LeftNavButton').click();
cy.findByTestId('LeftNav-CommandBarButtonDesign').should('exist');
cy.findByTestId('LeftNav-CommandBarButtonCreate').should('exist');
cy.findByTestId('LeftNav-CommandBarButtonBot responses').click();
cy.url().should('include', 'language-generation');
cy.findByTestId('LeftNav-CommandBarButtonUser input').click();
Expand Down
4 changes: 2 additions & 2 deletions Composer/cypress/integration/LuisDeploy.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,13 @@ context('Luis Deploy', () => {
});

it('can deploy luis success', () => {
cy.visitPage('Project settings');
cy.visitPage('Configure');
cy.findByText('Development resources').click();
cy.findAllByTestId('rootLUISAuthoringKey').type('12345678', { delay: 200 });
cy.findAllByTestId('rootLUISRegion').click();
cy.findByText('West US').click();
cy.visitPage('User input');
cy.visitPage('Design');
cy.visitPage('Create');
cy.route({
method: 'POST',
url: 'api/projects/*/build',
Expand Down
6 changes: 3 additions & 3 deletions Composer/cypress/integration/NotificationPage.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ context('Notification Page', () => {
});

it('can show lg syntax error ', () => {
cy.visitPage('Design');
cy.visitPage('Create');
cy.visitPage('Bot responses');

cy.findByTestId('ProjectTree').within(() => {
Expand All @@ -27,7 +27,7 @@ context('Notification Page', () => {
});

it('can show lu syntax error ', () => {
cy.visitPage('Design');
cy.visitPage('Create');
cy.visitPage('User input');

cy.findByTestId('ProjectTree').within(() => {
Expand All @@ -45,7 +45,7 @@ context('Notification Page', () => {
});

// it('can show dialog expression error ', () => {
// cy.visitPage('Design');
// cy.visitPage('Create');

// cy.findByTestId('ProjectTree').within(() => {
// cy.findByText('WelcomeUser').click();
Expand Down
4 changes: 2 additions & 2 deletions Composer/cypress/integration/Onboarding.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,12 +6,12 @@ context('Onboarding', () => {
cy.createTestBot('TestSample', ({ id }) => {
cy.visit(`/bot/${id}`);
//enable onboarding setting
cy.visitPage('Composer settings');
cy.visitPage('Composer settings', false);
cy.findByTestId('ProjectTree').within(() => {
cy.findByText('Application Settings').click();
});
cy.findByTestId('onboardingToggle').click();
cy.visitPage('Design');
cy.visitPage('Create');
});
});

Expand Down
2 changes: 1 addition & 1 deletion Composer/cypress/support/commands.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ declare namespace Cypress {
* Visits a page from the left nav bar using the page's testid
* @example visitPage('Bot Responses');
*/
visitPage(page: string): void;
visitPage(page: string, checked?: boolean): void;

/**
* Invokes callback inside editor context
Expand Down
4 changes: 2 additions & 2 deletions Composer/cypress/support/commands.ts
Original file line number Diff line number Diff line change
Expand Up @@ -83,9 +83,9 @@ Cypress.Commands.add('withinEditor', (editorName, cb) => {
cy.findByTestId(editorName).within(cb);
});

Cypress.Commands.add('visitPage', (page) => {
Cypress.Commands.add('visitPage', (page: string, checked = true) => {
cy.findByTestId(`LeftNav-CommandBarButton${page}`).click();
cy.findByTestId('ActiveLeftNavItem').should('contain', page);
if (checked) cy.findByTestId('ActiveLeftNavItem').should('contain', page);

// click the logo to clear any stray tooltips from page navigation
cy.findByAltText('Composer Logo').click({ force: true });
Expand Down
9 changes: 5 additions & 4 deletions Composer/packages/client/__tests__/components/header.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

import * as React from 'react';
import { within } from '@testing-library/dom';

import { renderWithRecoil } from '../testUtils';
import { Header } from '../../src/components/Header';
Expand All @@ -15,30 +16,30 @@ describe('<Header />', () => {
return { location: { pathname: 'http://server/home' } };
});
const { container } = renderWithRecoil(<Header />);
expect(container).toHaveTextContent('Bot Framework Composer');
expect(within(container).findByAltText('Composer Logo')).not.toBeNull();
});

it('should not show the start bots widget in Home page', async () => {
(useLocation as jest.Mock).mockImplementation(() => {
return { location: { pathname: 'http://server/home' } };
});
const { queryByText } = renderWithRecoil(<Header />);
expect(queryByText('Start all bots')).toBeNull();
expect(queryByText('Start all')).toBeNull();
});

it('should show the start bots widget on design page', async () => {
(useLocation as jest.Mock).mockImplementation(() => {
return { location: { pathname: 'http://server/bot/1234/dialogs' } };
});
const result = renderWithRecoil(<Header />);
expect(result.findAllByDisplayValue('Start all bots')).not.toBeNull();
expect(result.findAllByDisplayValue('Start all')).not.toBeNull();
});

it('should show the start bots widget on settings page', async () => {
(useLocation as jest.Mock).mockImplementation(() => {
return { location: { pathname: 'http://server/bot/1234/settings' } };
});
const result = renderWithRecoil(<Header />);
expect(result.findAllByDisplayValue('Start all bots')).not.toBeNull();
expect(result.findAllByDisplayValue('Start all')).not.toBeNull();
});
});
3 changes: 2 additions & 1 deletion Composer/packages/client/__tests__/routers.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

import React from 'react';
import { within } from '@testing-library/dom';
import { render } from '@botframework-composer/test-utils';
import { createHistory, createMemorySource, LocationProvider } from '@reach/router';

Expand Down Expand Up @@ -35,7 +36,7 @@ describe('<Router/> router test', () => {
} = renderWithRouter(wrapWithRecoil(<AppTest />));

const appContainer = container;
expect(appContainer.innerHTML).toMatch('Bot Framework Composer');
expect(within(appContainer).findByAltText('Composer Logo')).not.toBeNull();

navigate('/language-understanding');
expect(appContainer.innerHTML).toMatch('Setting');
Expand Down
6 changes: 3 additions & 3 deletions Composer/packages/client/src/Onboarding/content.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,9 +81,9 @@ export const stepSets = (projectId: string, rootDialogId: string): IStepSet[] =>
id: 'publish',
steps: [
{
id: 'projectSettings',
id: 'publish',
navigateTo: `/bot/${projectId}/dialogs/${rootDialogId}?selected=triggers[0]`,
targetId: 'navProjectsettings',
targetId: 'navPublish',
},
],
title: formatMessage('Configure and publish'),
Expand Down Expand Up @@ -120,7 +120,7 @@ export const getTeachingBubble = (id: string | undefined): TeachingBubble => {
case 'actions':
return {
content: formatMessage(
'Actions are the main component of a trigger, they are what enable your bot to take action whether in response to user input or any other event that may occur.'
'Actions are the main component of a trigger; they are what enable your bot to take action whether in response to user input or any other event that may occur.'
),
headline: formatMessage('Actions'),
helpLink: 'https://docs.microsoft.com/en-us/composer/concept-dialog#action',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,7 +161,7 @@ const BotController: React.FC<BotControllerProps> = ({ onHideController, isContr
`{
total, plural,
=1 {Start bot}
other {Start all bots}
other {Start all}
}`,
{ total: runningBots.totalBots }
)
Expand Down Expand Up @@ -275,7 +275,7 @@ const BotController: React.FC<BotControllerProps> = ({ onHideController, isContr
</DisableFeatureToolTip>
<div ref={onboardRef} css={[iconSectionContainer, disableStartBots ? disabledStyle : '']}>
<IconButton
ariaDescription={formatMessage('Open start bots panel')}
ariaDescription={formatMessage('Start and stop local bot runtimes')}
data-testid="StartBotsPanel"
disabled={disableStartBots}
iconProps={{
Expand All @@ -296,7 +296,7 @@ const BotController: React.FC<BotControllerProps> = ({ onHideController, isContr
},
rootHovered: { background: transparentBackground, color: NeutralColors.white },
}}
title={formatMessage('Open start bots panel')}
title={formatMessage('Start and stop local bot runtimes')}
onClick={onSplitButtonClick}
/>
</div>
Expand Down
44 changes: 15 additions & 29 deletions Composer/packages/client/src/components/Header.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import { FocusTrapZone } from 'office-ui-fabric-react/lib/FocusTrapZone';
import { useCallback, useState, Fragment, useMemo, useEffect } from 'react';
import { NeutralColors, SharedColors, FontSizes, CommunicationColors } from '@uifabric/fluent-theme';
import { useRecoilValue } from 'recoil';
import { FontWeights } from 'office-ui-fabric-react/lib/Styling';
import { TeachingBubble } from 'office-ui-fabric-react/lib/TeachingBubble';

import { useLocation } from '../utils/hooks';
Expand Down Expand Up @@ -52,13 +51,6 @@ const headerContainer = css`
align-items: center;
`;

const title = css`
margin-left: 20px;
font-weight: ${FontWeights.semibold};
font-size: ${FontSizes.size16};
color: #fff;
`;

const botName = css`
font-size: 16px;
color: #fff;
Expand All @@ -76,12 +68,6 @@ const botLocale = css`
cursor: pointer;
`;

const divider = css`
height: 24px;
border-right: 1px solid #979797;
margin: 0px 0px 0px 20px;
`;

const headerTextContainer = css`
display: flex;
align-items: center;
Expand Down Expand Up @@ -272,21 +258,21 @@ export const Header = () => {
style={{ marginLeft: '9px' }}
/>
<div css={headerTextContainer}>
<div css={title}>{formatMessage('Bot Framework Composer')}</div>
{projectName && (
<Fragment>
<div css={divider} />
<span css={botName}>{projectName}</span>
<span
css={botLocale}
id="targetButton"
role={'button'}
tabIndex={0}
onClick={() => setTeachingBubbleVisibility(true)}
onKeyDown={handleActiveLanguageButtonOnKeyDown}
>
{languageFullName(locale)}
</span>
{languageListOptions.length > 1 && (
<span
css={botLocale}
id="targetButton"
role={'button'}
tabIndex={0}
onClick={() => setTeachingBubbleVisibility(true)}
onKeyDown={handleActiveLanguageButtonOnKeyDown}
>
{languageFullName(locale)}
</span>
)}
</Fragment>
)}
</div>
Expand All @@ -311,13 +297,13 @@ export const Header = () => {
)}
{isShow && (
<IconButton
ariaDescription={formatMessage('Test in web chat')}
ariaDescription={formatMessage('Test your bot')}
disabled={!webchatEssentials?.botUrl}
iconProps={{
iconName: 'OfficeChat',
}}
styles={buttonStyles}
title={formatMessage('Test in Web Chat')}
title={formatMessage('Test your bot')}
onClick={() => {
const currentWebChatVisibility = !isWebChatPanelVisible;
setWebChatPanelVisibility(currentWebChatVisibility);
Expand All @@ -335,7 +321,7 @@ export const Header = () => {
iconProps={{ iconName: 'Rocket' }}
id="rocketButton"
styles={buttonStyles}
title={formatMessage('Get started')}
title={formatMessage('Recommended actions')}
onClick={() => toggleGetStarted(true)}
/>
)}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ describe('<BotController />', () => {
<BotController isControllerHidden={false} onHideController={jest.fn()} />,
initRecoilState
);
await findByText('Start all bots');
await findByText('Start all');
});

it('should stop all bots if Stop all bots is clicked', async () => {
Expand Down
44 changes: 22 additions & 22 deletions Composer/packages/client/src/utils/pageLinks.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,17 +42,17 @@ export const topLinks = (
{
to: linkBase + `dialogs/${openedDialogId}`,
iconName: 'SplitObject',
labelName: formatMessage('Design'),
labelName: formatMessage('Create'),
disabled: !botLoaded,
match: /(bot\/[0-9.]+)$|(bot\/[0-9.]+\/skill\/[0-9.]+)$/,
isDisabledForPVA: false,
},
{
to: linkBase + `language-generation/${openedDialogId}`,
iconName: 'Robot',
labelName: formatMessage('Bot responses'),
disabled: !botLoaded || skillIsRemote,
match: /language-generation\/[a-zA-Z0-9_-]+$/,
to: `/bot/${rootProjectId || projectId}/botProjectsSettings`,
iconName: 'BotProjectsSettings',
labelName: formatMessage('Configure'),
disabled: !botLoaded,
match: /botProjectsSettings/,
isDisabledForPVA: false,
},
{
Expand All @@ -63,29 +63,22 @@ export const topLinks = (
match: /language-understanding\/[a-zA-Z0-9_-]+$/,
isDisabledForPVA: false,
},
{
to: linkBase + `language-generation/${openedDialogId}`,
iconName: 'Robot',
labelName: formatMessage('Bot responses'),
disabled: !botLoaded || skillIsRemote,
match: /language-generation\/[a-zA-Z0-9_-]+$/,
isDisabledForPVA: false,
},
{
to: linkBase + `knowledge-base/${openedDialogId}`,
iconName: 'QnAIcon',
labelName: formatMessage('QnA'),
labelName: formatMessage('Knowledge base'),
disabled: !botLoaded || skillIsRemote,
match: /knowledge-base\/[a-zA-Z0-9_-]+$/,
isDisabledForPVA: isPVASchema,
},
{
to: `/bot/${rootProjectId || projectId}/publish`,
iconName: 'CloudUpload',
labelName: formatMessage('Publish'),
disabled: !botLoaded,
isDisabledForPVA: false,
},
{
to: `/bot/${rootProjectId || projectId}/botProjectsSettings`,
iconName: 'BotProjectsSettings',
labelName: formatMessage('Project settings'),
disabled: !botLoaded,
match: /botProjectsSettings/,
isDisabledForPVA: false,
},
...(showFormDialog
? [
{
Expand All @@ -97,6 +90,13 @@ export const topLinks = (
},
]
: []),
{
to: `/bot/${rootProjectId || projectId}/publish`,
iconName: 'CloudUpload',
labelName: formatMessage('Publish'),
disabled: !botLoaded,
isDisabledForPVA: false,
},
];

if (pluginPages.length > 0) {
Expand Down
Loading

0 comments on commit c66c44d

Please sign in to comment.