From f491f539eb60ca42928a236932e2aa91d0cc6ab2 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Fri, 7 Aug 2020 11:28:18 -0700 Subject: [PATCH 01/15] ability to update google analytics from settings view. adding settings view to app with route --- .../src/dashboard/app/api/useSettingsApi.js | 8 ++++--- assets/src/dashboard/app/index.js | 5 +++++ assets/src/dashboard/app/reducer/settings.js | 4 ++-- .../editorSettings/googleAnalytics/index.js | 18 +++++++++------ .../app/views/editorSettings/index.js | 22 +++++++++++++++++-- includes/Dashboard.php | 1 + 6 files changed, 44 insertions(+), 14 deletions(-) diff --git a/assets/src/dashboard/app/api/useSettingsApi.js b/assets/src/dashboard/app/api/useSettingsApi.js index 44f67c02f91c..ebc478bc1ae1 100644 --- a/assets/src/dashboard/app/api/useSettingsApi.js +++ b/assets/src/dashboard/app/api/useSettingsApi.js @@ -76,18 +76,20 @@ export default function useSettingsApi( }, [dataAdapter, globalStoriesSettingsApi]); const updateSettings = useCallback( - async (settings = {}) => { + async ({ googleAnalyticsId }) => { try { const response = await dataAdapter.post( queryString.stringifyUrl({ url: globalStoriesSettingsApi, - query: settings, + query: { + web_stories_ga_tracking_id: googleAnalyticsId, + }, }) ); dispatch({ type: SETTINGS_ACTION_TYPES.UPDATE_SETTINGS_SUCCESS, - payload: response, + payload: { googleAnalyticsId: response.web_stories_ga_tracking_id }, }); } catch (err) { dispatch({ diff --git a/assets/src/dashboard/app/index.js b/assets/src/dashboard/app/index.js index 3a5c234c3353..09b6bd40c5dd 100644 --- a/assets/src/dashboard/app/index.js +++ b/assets/src/dashboard/app/index.js @@ -50,6 +50,7 @@ import ApiProvider from './api/apiProvider'; import { Route, RouterProvider, RouterContext, matchPath } from './router'; import { ConfigProvider } from './config'; import { + EditorSettingsView, ExploreTemplatesView, MyStoriesView, SavedTemplatesView, @@ -104,6 +105,10 @@ const AppContent = () => { path={NESTED_APP_ROUTES.SAVED_TEMPLATE_DETAIL} component={} /> + } + /> } diff --git a/assets/src/dashboard/app/reducer/settings.js b/assets/src/dashboard/app/reducer/settings.js index 050d8a8c1f79..5d31c6dedd78 100644 --- a/assets/src/dashboard/app/reducer/settings.js +++ b/assets/src/dashboard/app/reducer/settings.js @@ -23,7 +23,7 @@ export const ACTION_TYPES = { export const defaultSettingsState = { error: {}, - googleAnalyticsId: null, + googleAnalyticsId: '', publisherLogos: [], }; @@ -43,7 +43,7 @@ function settingsReducer(state, action) { ...state, error: {}, googleAnalyticsId: action.payload.googleAnalyticsId, - publisherLogos: action.payload.publisherLogos, + publisherLogos: action.payload.publisherLogos || [], }; } diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index d815547e4796..9c61add204bb 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -48,7 +48,8 @@ const TEXT = { function GoogleAnalyticsSettings({ googleAnalyticsId = '', - onUpdateGoogleAnalyticsId, + handleUpdateSettings, + canManageSettings, }) { const [analyticsId, setAnalyticsId] = useState(googleAnalyticsId); @@ -56,11 +57,12 @@ function GoogleAnalyticsSettings({ setAnalyticsId(googleAnalyticsId); }, [googleAnalyticsId]); - const handleCompleteUpdateId = useCallback( - (newId) => { - onUpdateGoogleAnalyticsId({ googleAnalyticsId: newId }); + const handleUpdateId = useCallback( + (value) => { + // todo add validation to string format + handleUpdateSettings({ newGoogleAnalyticsId: value }); }, - [onUpdateGoogleAnalyticsId] + [handleUpdateSettings] ); return ( @@ -74,8 +76,9 @@ function GoogleAnalyticsSettings({ id="gaTrackingId" value={analyticsId} onEditCancel={handleCancelUpdateId} - onEditComplete={handleCompleteUpdateId} + onEditComplete={handleUpdateId} placeholder={TEXT.PLACEHOLDER} + disabled={!canManageSettings} /> {TEXT.CONTEXT} @@ -83,7 +86,8 @@ function GoogleAnalyticsSettings({ ); } GoogleAnalyticsSettings.propTypes = { - onUpdateGoogleAnalyticsId: PropTypes.func, + handleUpdateSettings: PropTypes.func, + canManageSettings: PropTypes.bool, googleAnalyticsId: PropTypes.string, }; diff --git a/assets/src/dashboard/app/views/editorSettings/index.js b/assets/src/dashboard/app/views/editorSettings/index.js index dc97dad3de74..76a9f3a4cdfb 100644 --- a/assets/src/dashboard/app/views/editorSettings/index.js +++ b/assets/src/dashboard/app/views/editorSettings/index.js @@ -21,17 +21,22 @@ import { __ } from '@wordpress/i18n'; /** * External dependencies */ -import { useContext, useEffect } from 'react'; +import { useCallback, useContext, useEffect } from 'react'; /** * Internal dependencies */ import { ApiContext } from '../../api/apiProvider'; +import { useConfig } from '../../config'; import GoogleAnalyticsSettings from './googleAnalytics'; import PublisherLogoSettings from './publisherLogo'; import { Wrapper, Header, Heading, Main } from './components'; function EditorSettings() { + const { + capabilities: { canManageSettings }, + } = useConfig(); + const { actions: { settingsApi: { fetchSettings, updateSettings }, @@ -45,6 +50,18 @@ function EditorSettings() { fetchSettings(); }, [fetchSettings]); + const handleCompleteUpdateId = useCallback( + ({ newGoogleAnalyticsId }) => { + const updatedSettings = { + googleAnalyticsId: + typeof newGoogleAnalyticsId === 'string' && newGoogleAnalyticsId, + }; + + updateSettings(updatedSettings); + }, + [updateSettings] + ); + return (
@@ -52,8 +69,9 @@ function EditorSettings() {
'/wp/v2/users', 'fonts' => '/web-stories/v1/fonts', 'templates' => '/web-stories/v1/web-story-template', + 'settings' => '/wp/v2/settings', ], 'maxUpload' => $max_upload_size, 'capabilities' => [ From a0d8aa6cd9ab7aaa4d3b4ce19efca89f16593e3d Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Fri, 7 Aug 2020 13:58:13 -0700 Subject: [PATCH 02/15] adding ability for inline input to be disabled. adding unit tests for settings --- .../editorSettings/googleAnalytics/index.js | 2 +- .../googleAnalytics/stories/index.js | 10 +++ .../googleAnalytics/test/googleAnalytics.js | 61 ++++++++++++++ .../editorSettings/test/editorSettings.js | 79 +++++++++++++++++++ .../components/inlineInputForm/index.js | 3 + 5 files changed, 154 insertions(+), 1 deletion(-) create mode 100644 assets/src/dashboard/app/views/editorSettings/googleAnalytics/test/googleAnalytics.js create mode 100644 assets/src/dashboard/app/views/editorSettings/test/editorSettings.js diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index 9c61add204bb..5456d7a16f8e 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -36,7 +36,7 @@ import { TextInputHelperText, } from '../components'; -const TEXT = { +export const TEXT = { CONTEXT: __( "The story editor will append a default, configurable AMP analytics configuration to your story. If you're interested in going beyond what the default configuration is, read this article.", 'web-stories' diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js index ed0800ddb9ac..059afb8a665a 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js @@ -31,6 +31,16 @@ export default { }; export const _default = () => { + return ( + + ); +}; + +export const _CannotManageSettings = () => { return ( ', function () { + const mockUpdate = jest.fn(); + + it('should render google analytics input and helper text by default', function () { + const { getByRole, getByText } = renderWithTheme( + + ); + + const input = getByRole('textbox'); + expect(input).toBeDefined(); + + const sectionHeader = getByText(TEXT.SECTION_HEADING); + expect(sectionHeader).toBeInTheDocument(); + }); + + it('should call mockUpdate when enter is keyed on input', function () { + const { getByRole } = renderWithTheme( + + ); + + const input = getByRole('textbox'); + + fireEvent.keyDown(input, { key: 'enter', keyCode: 13 }); + + expect(mockUpdate).toHaveBeenCalledTimes(1); + }); +}); diff --git a/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js new file mode 100644 index 000000000000..a801ff26ce77 --- /dev/null +++ b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js @@ -0,0 +1,79 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +/** + * External dependencies + */ +import PropTypes from 'prop-types'; + +/** + * Internal dependencies + */ +import { renderWithTheme } from '../../../../testUtils'; +import EditorSettings from '../'; +import { ApiContext } from '../../../api/apiProvider'; +import { ConfigProvider } from '../../../config'; + +const mockFetchSettings = jest.fn(); + +const SettingsWrapper = ({ canManageSettings, googleAnalyticsId }) => { + return ( + + + + + + ); +}; + +SettingsWrapper.propTypes = { + googleAnalyticsId: PropTypes.string, + canManageSettings: PropTypes.bool, +}; + +describe('Editor Settings: ', function () { + it('should render settings page with google analytics and publisher logo sections', function () { + const { getByText, getByRole } = renderWithTheme( + + ); + + const googleAnalyticsHeading = getByText('Google Analytics Tracking ID'); + expect(googleAnalyticsHeading).toBeInTheDocument(); + + const publisherLogoHeading = getByText('Publisher Logo'); + expect(publisherLogoHeading).toBeInTheDocument(); + + const input = getByRole('textbox'); + expect(input).toBeDefined(); + + expect(input.value).toBe('123-45-98-not-an-id'); + + expect(mockFetchSettings).toHaveBeenCalledTimes(1); + }); +}); diff --git a/assets/src/dashboard/components/inlineInputForm/index.js b/assets/src/dashboard/components/inlineInputForm/index.js index 64a24148d963..99d89bdf00a4 100644 --- a/assets/src/dashboard/components/inlineInputForm/index.js +++ b/assets/src/dashboard/components/inlineInputForm/index.js @@ -36,6 +36,7 @@ const InlineInputForm = ({ onEditComplete, placeholder, value, + disabled, }) => { const inputContainerRef = useRef(null); const [newValue, setNewValue] = useState(value); @@ -79,12 +80,14 @@ const InlineInputForm = ({ onKeyDown={handleKeyPress} onChange={handleChange} placeholder={placeholder} + disabled={disabled} /> ); }; InlineInputForm.propTypes = { + disabled: PropTypes.bool, id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, label: PropTypes.string.isRequired, onEditCancel: PropTypes.func.isRequired, From a1f7f0cd8a0fa859eb65c68a99026b6c8777b826 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Mon, 10 Aug 2020 10:27:17 -0700 Subject: [PATCH 03/15] adding karma tests for settings as well as config for karma for settings. --- .../app/views/editorSettings/index.js | 6 +- .../karma/editorSettings.karma.js | 94 +++++++++++++++++++ .../src/dashboard/karma/apiProviderFixture.js | 53 ++++++++++- assets/src/dashboard/karma/fixture.js | 4 + 4 files changed, 151 insertions(+), 6 deletions(-) create mode 100644 assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js diff --git a/assets/src/dashboard/app/views/editorSettings/index.js b/assets/src/dashboard/app/views/editorSettings/index.js index 76a9f3a4cdfb..237190787682 100644 --- a/assets/src/dashboard/app/views/editorSettings/index.js +++ b/assets/src/dashboard/app/views/editorSettings/index.js @@ -33,9 +33,7 @@ import PublisherLogoSettings from './publisherLogo'; import { Wrapper, Header, Heading, Main } from './components'; function EditorSettings() { - const { - capabilities: { canManageSettings }, - } = useConfig(); + const { capabilities: { canManageSettings } = {} } = useConfig(); const { actions: { @@ -63,7 +61,7 @@ function EditorSettings() { ); return ( - +
{__('Settings', 'web-stories')}
diff --git a/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js b/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js new file mode 100644 index 000000000000..8cb86e9a0726 --- /dev/null +++ b/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js @@ -0,0 +1,94 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * External dependencies + */ +import { within } from '@testing-library/react'; + +/** + * Internal dependencies + */ +import { useContext } from 'react'; +import Fixture from '../../../../karma/fixture'; +import { ApiContext } from '../../../api/apiProvider'; + +describe('Settings View', () => { + let fixture; + + beforeEach(async () => { + fixture = new Fixture(); + + await fixture.render(); + + await navigateToEditorSettings(); + }); + + afterEach(() => { + fixture.restore(); + }); + + function navigateToEditorSettings() { + const editorSettingsMenuItem = fixture.screen.queryByRole('link', { + name: /^Editor Settings$/, + }); + return fixture.events.click(editorSettingsMenuItem); + } + + async function getSettingsState() { + const { + state: { settings }, + } = await fixture.renderHook(() => useContext(ApiContext)); + + return settings; + } + + it('should render', async () => { + const settingsView = await fixture.screen.getByTestId('editor-settings'); + + const PageHeading = within(settingsView).getByText('Settings'); + + expect(PageHeading).toBeTruthy(); + }); + + it('should update the tracking id', async () => { + const settingsView = await fixture.screen.getByTestId('editor-settings'); + + const input = within(settingsView).getByRole('textbox'); + + await fixture.events.hover(input); + + await fixture.events.click(input); + + const inputLength = input.value.length; + + for (let iter = 0; iter < inputLength; iter++) { + // disable eslint to prevent overlapping .act calls + // eslint-disable-next-line no-await-in-loop + await fixture.events.keyboard.press('Backspace'); + } + + await fixture.events.keyboard.type('UA-009345-6'); + await fixture.events.keyboard.press('Enter'); + + const { googleAnalyticsId } = await getSettingsState(); + + const newInput = await fixture.screen.getByRole('textbox'); + expect(newInput.value).toBe(googleAnalyticsId); + }); + + // it("it should not allow an update of google analytics id when id format doesn't match required format", () => {}); +}); diff --git a/assets/src/dashboard/karma/apiProviderFixture.js b/assets/src/dashboard/karma/apiProviderFixture.js index 94e84c17fe96..b0f135e7d1a4 100644 --- a/assets/src/dashboard/karma/apiProviderFixture.js +++ b/assets/src/dashboard/karma/apiProviderFixture.js @@ -34,16 +34,29 @@ import { STORY_STATUSES, STORY_SORT_OPTIONS } from '../constants/stories'; /* eslint-disable jasmine/no-unsafe-spy */ export default function ApiProviderFixture({ children }) { + const [settings, setSettingsState] = useState(getSettingsState()); const [stories, setStoriesState] = useState(getStoriesState()); const [templates, setTemplatesState] = useState(getTemplatesState()); const [users] = useState(formattedUsersObject); + const settingsApi = useMemo( + () => ({ + fetchSettings: () => + setSettingsState((currentState) => fetchSettings(currentState)), + updateSettings: (updates) => + setSettingsState((currentState) => + updateSettings(updates, currentState) + ), + }), + [] + ); + const storyApi = useMemo( () => ({ duplicateStory: (story) => setStoriesState((currentState) => duplicateStory(story, currentState)), fetchStories: (...args) => - setStoriesState((currenState) => fetchStories(...args, currenState)), + setStoriesState((currentState) => fetchStories(...args, currentState)), clearStoryPreview: () => setStoriesState((currentState) => clearStoryPreview(currentState)), createStoryPreview: () => @@ -101,18 +114,30 @@ export default function ApiProviderFixture({ children }) { const value = useMemo( () => ({ state: { + settings, stories, templates, users, }, actions: { + settingsApi, storyApi, templateApi, fontApi, usersApi, }, }), - [stories, templates, users, usersApi, storyApi, templateApi, fontApi] + [ + settings, + stories, + templates, + users, + settingsApi, + storyApi, + templateApi, + fontApi, + usersApi, + ] ); return {children}; @@ -123,6 +148,30 @@ ApiProviderFixture.propTypes = { children: PropTypes.node, }; +function getSettingsState() { + return { + error: {}, + googleAnalyticsId: '', + publisherLogos: [], + }; +} + +function fetchSettings(currentState) { + const settingsState = { ...currentState } || getSettingsState(); + settingsState.googleAnalyticsId = 'UA-000000-2'; + + return settingsState; +} + +function updateSettings(updates, currentState) { + const { googleAnalyticsId } = updates; + + return { + ...currentState, + googleAnalyticsId, + }; +} + function getStoriesState() { const copiedStories = [...formattedStoriesArray]; copiedStories.sort((a, b) => b.modified.diff(a.modified)); // initial sort is desc by modified diff --git a/assets/src/dashboard/karma/fixture.js b/assets/src/dashboard/karma/fixture.js index 756b60acca55..25f9a530e636 100644 --- a/assets/src/dashboard/karma/fixture.js +++ b/assets/src/dashboard/karma/fixture.js @@ -34,6 +34,9 @@ import { AppFrame } from '../components'; import ApiProviderFixture from './apiProviderFixture'; const defaultConfig = { + capabilities: { + canManageSettings: true, + }, isRTL: false, dateFormat: 'F j, Y', timeFormat: 'g:i a', @@ -49,6 +52,7 @@ const defaultConfig = { stories: '/web-stories/v1/web-story', users: '/wp/v2/users', fonts: '/web-stories/v1/fonts', + settings: '/wp/v2/settings', }, }; From f60cda7728714555381ebca717f8f1863bb51b16 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Mon, 10 Aug 2020 12:32:35 -0700 Subject: [PATCH 04/15] basic regex to validate the format of the entered ID before sending off to API, update input to show error structure --- .../editorSettings/googleAnalytics/index.js | 11 ++++- .../googleAnalytics/test/googleAnalytics.js | 13 ++++++ .../components/inlineInputForm/index.js | 12 +++++ .../inlineInputForm/stories/index.js | 2 + .../src/dashboard/components/input/index.js | 6 ++- assets/src/dashboard/theme.js | 1 + assets/src/dashboard/utils/index.js | 1 + .../test/validateGoogleAnalyticsIdFormat.js | 45 +++++++++++++++++++ .../utils/validateGoogleAnalyticsIdFormat.js | 20 +++++++++ 9 files changed, 107 insertions(+), 4 deletions(-) create mode 100644 assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js create mode 100644 assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index 5456d7a16f8e..642aadc08f12 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -28,6 +28,7 @@ import PropTypes from 'prop-types'; /** * Internal dependencies */ +import { validateGoogleAnalyticsIdFormat } from '../../../../utils'; import { InlineInputForm } from '../../../../components'; import { FormContainer, @@ -52,6 +53,7 @@ function GoogleAnalyticsSettings({ canManageSettings, }) { const [analyticsId, setAnalyticsId] = useState(googleAnalyticsId); + const [inputError, setInputError] = useState(''); const handleCancelUpdateId = useCallback(() => { setAnalyticsId(googleAnalyticsId); @@ -60,9 +62,13 @@ function GoogleAnalyticsSettings({ const handleUpdateId = useCallback( (value) => { // todo add validation to string format - handleUpdateSettings({ newGoogleAnalyticsId: value }); + if (value.length === 0 || validateGoogleAnalyticsIdFormat(value)) { + setInputError(''); + return handleUpdateSettings({ newGoogleAnalyticsId: value }); + } + return setInputError('Invalid ID format'); }, - [handleUpdateSettings] + [handleUpdateSettings, setInputError] ); return ( @@ -79,6 +85,7 @@ function GoogleAnalyticsSettings({ onEditComplete={handleUpdateId} placeholder={TEXT.PLACEHOLDER} disabled={!canManageSettings} + error={inputError} /> {TEXT.CONTEXT} diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/test/googleAnalytics.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/test/googleAnalytics.js index ccd656c90d69..b0b1de0209b1 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/test/googleAnalytics.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/test/googleAnalytics.js @@ -53,9 +53,22 @@ describe('Editor Settings: Google Analytics ', function () { ); const input = getByRole('textbox'); + fireEvent.change(input, { target: { value: 'UA-098754-33' } }); fireEvent.keyDown(input, { key: 'enter', keyCode: 13 }); expect(mockUpdate).toHaveBeenCalledTimes(1); + + fireEvent.change(input, { target: { value: '' } }); + + fireEvent.keyDown(input, { key: 'enter', keyCode: 13 }); + + expect(mockUpdate).toHaveBeenCalledTimes(2); + + fireEvent.change(input, { target: { value: 'NOT A VALID ID!!!' } }); + + fireEvent.keyDown(input, { key: 'enter', keyCode: 13 }); + + expect(mockUpdate).toHaveBeenCalledTimes(2); }); }); diff --git a/assets/src/dashboard/components/inlineInputForm/index.js b/assets/src/dashboard/components/inlineInputForm/index.js index 99d89bdf00a4..4d37f28c2851 100644 --- a/assets/src/dashboard/components/inlineInputForm/index.js +++ b/assets/src/dashboard/components/inlineInputForm/index.js @@ -26,10 +26,19 @@ import styled from 'styled-components'; import { visuallyHiddenStyles } from '../../utils/visuallyHiddenStyles'; import { useFocusOut } from '../../utils/'; import { TextInput } from '../input'; +import { TypographyPresets } from '../typography'; const Label = styled.label(visuallyHiddenStyles); +const ErrorText = styled.p` + ${TypographyPresets.ExtraSmall}; + color: ${({ theme }) => theme.colors.danger}; + margin-left: 1em; + padding-top: 0.25em; +`; + const InlineInputForm = ({ + error, id, label, onEditCancel, @@ -81,13 +90,16 @@ const InlineInputForm = ({ onChange={handleChange} placeholder={placeholder} disabled={disabled} + error={error} /> + {error && {error}} ); }; InlineInputForm.propTypes = { disabled: PropTypes.bool, + error: PropTypes.string, id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, label: PropTypes.string.isRequired, onEditCancel: PropTypes.func.isRequired, diff --git a/assets/src/dashboard/components/inlineInputForm/stories/index.js b/assets/src/dashboard/components/inlineInputForm/stories/index.js index 741731344c62..e0a81bb109ce 100644 --- a/assets/src/dashboard/components/inlineInputForm/stories/index.js +++ b/assets/src/dashboard/components/inlineInputForm/stories/index.js @@ -18,6 +18,7 @@ * External dependencies */ import { action } from '@storybook/addon-actions'; +import { text } from '@storybook/addon-knobs'; /** * Internal dependencies @@ -36,5 +37,6 @@ export const _default = () => ( value={'some input value'} id={'898989'} label="my hidden input label" + error={text('error', '')} /> ); diff --git a/assets/src/dashboard/components/input/index.js b/assets/src/dashboard/components/input/index.js index 919bbf399df0..7daae1bb8c6f 100644 --- a/assets/src/dashboard/components/input/index.js +++ b/assets/src/dashboard/components/input/index.js @@ -29,8 +29,10 @@ export const TextInput = styled.input` margin: 0; padding: 1px 8px; border-radius: 6px; - border: ${({ theme }) => theme.borders.gray100}; + border: ${({ theme, error }) => + error ? theme.borders.danger : theme.borders.gray100}; &:active { - border: ${({ theme }) => theme.borders.action}; + border: ${({ theme, error }) => + error ? theme.borders.danger : theme.borders.action}; } `; diff --git a/assets/src/dashboard/theme.js b/assets/src/dashboard/theme.js index 63f2c9eeee50..0c3b76e91ab0 100644 --- a/assets/src/dashboard/theme.js +++ b/assets/src/dashboard/theme.js @@ -84,6 +84,7 @@ const borders = { transparent: '1px solid transparent', bluePrimary: `1px solid ${colors.bluePrimary}`, action: `1px solid ${colors.action}`, + danger: `1px solid ${colors.danger}`, }; const theme = { diff --git a/assets/src/dashboard/utils/index.js b/assets/src/dashboard/utils/index.js index 2bcfb81578b6..adfc5e335023 100644 --- a/assets/src/dashboard/utils/index.js +++ b/assets/src/dashboard/utils/index.js @@ -29,6 +29,7 @@ export { } from './usePagePreviewSize'; export { default as useStoryView } from './useStoryView'; export { default as useTemplateView } from './useTemplateView'; +export { default as validateGoogleAnalyticsIdFormat } from './validateGoogleAnalyticsIdFormat'; export { default as addQueryArgs } from '../../edit-story/utils/addQueryArgs'; export { default as getStoryPropsToSave } from '../../edit-story/app/story/utils/getStoryPropsToSave'; diff --git a/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js b/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js new file mode 100644 index 000000000000..03c3902e06df --- /dev/null +++ b/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js @@ -0,0 +1,45 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +/** + * Internal dependencies + */ +import { validateGoogleAnalyticsIdFormat } from '../'; + +const validIdFormats = [ + ['UA-000000-56', true], + ['ua-098765432-9875', true], +]; +const invalidIdFormats = [ + ['78787878', false], + ['ua--123448-0', false], + ['clearly wrong', false], +]; + +describe('validateGoogleAnalyticsIdFormat', () => { + it.each(validIdFormats)('should return %s as true', (validId, expected) => { + const bool = validateGoogleAnalyticsIdFormat(validId); + expect(bool).toBe(expected); + }); + + it.each(invalidIdFormats)( + 'should return %s as false', + (invalidId, expected) => { + const bool = validateGoogleAnalyticsIdFormat(invalidId); + expect(bool).toBe(expected); + } + ); +}); diff --git a/assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js b/assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js new file mode 100644 index 000000000000..081ed335df18 --- /dev/null +++ b/assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js @@ -0,0 +1,20 @@ +/* + * Copyright 2020 Google LLC + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * https://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +export default function validateGoogleAnalyticsIdFormat(value = '') { + const googleAnalyticsIdFormatRegex = /^ua-\d+-\d+$/; + return Boolean(value.toLowerCase().match(googleAnalyticsIdFormatRegex)); +} From 2eedef4d3f4cb85bcc73ea2f8881067302f34961 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Mon, 10 Aug 2020 12:40:27 -0700 Subject: [PATCH 05/15] fixing double form submit while keeping proper tags. fixing heading hierachy --- .../dashboard/app/views/editorSettings/googleAnalytics/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index 642aadc08f12..e5c3f5c13727 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -72,7 +72,7 @@ function GoogleAnalyticsSettings({ ); return ( - + e.preventDefault()}> {TEXT.SECTION_HEADING} From 3ad626ba816dec1db6d183f575a57611f5d0d2d5 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Mon, 10 Aug 2020 13:34:49 -0700 Subject: [PATCH 06/15] adding a new flag for enabling settings view since it is functional but needs testing before being allowed to be used. prepping settings page to be usable by clearing out the stubbed publisher logo component (that will follow as a separate PR under the same flag). Adjusting the secondary app routes to conditionally allow editor settings to be visible. Have to check flag at app level too to allow SettingsView to be available. --- assets/src/dashboard/app/index.js | 13 +++++++++---- .../dashboard/app/views/editorSettings/index.js | 7 +------ .../editorSettings/karma/editorSettings.karma.js | 1 + .../dashboard/components/pageStructure/index.js | 15 ++++++++++++--- assets/src/dashboard/constants/index.js | 1 - includes/Experiments.php | 11 +++++++++++ 6 files changed, 34 insertions(+), 14 deletions(-) diff --git a/assets/src/dashboard/app/index.js b/assets/src/dashboard/app/index.js index 09b6bd40c5dd..10e32b0ddb8c 100644 --- a/assets/src/dashboard/app/index.js +++ b/assets/src/dashboard/app/index.js @@ -26,6 +26,7 @@ import { useContext, useEffect } from 'react'; import { StyleSheetManager, ThemeProvider } from 'styled-components'; import stylisRTLPlugin from 'stylis-plugin-rtl'; import PropTypes from 'prop-types'; +import { useFeature } from 'flagged'; /** * Internal dependencies @@ -64,6 +65,8 @@ const AppContent = () => { state: { currentPath }, } = useContext(RouterContext); + const enableSettingsView = useFeature('enableSettingsView'); + useEffect(() => { const dynamicPageTitle = ROUTE_TITLES[currentPath] || ROUTE_TITLES.DEFAULT; window.document.title = sprintf( @@ -105,10 +108,12 @@ const AppContent = () => { path={NESTED_APP_ROUTES.SAVED_TEMPLATE_DETAIL} component={} /> - } - /> + {enableSettingsView && ( + } + /> + )} } diff --git a/assets/src/dashboard/app/views/editorSettings/index.js b/assets/src/dashboard/app/views/editorSettings/index.js index 237190787682..1048ce4e896a 100644 --- a/assets/src/dashboard/app/views/editorSettings/index.js +++ b/assets/src/dashboard/app/views/editorSettings/index.js @@ -29,7 +29,6 @@ import { useCallback, useContext, useEffect } from 'react'; import { ApiContext } from '../../api/apiProvider'; import { useConfig } from '../../config'; import GoogleAnalyticsSettings from './googleAnalytics'; -import PublisherLogoSettings from './publisherLogo'; import { Wrapper, Header, Heading, Main } from './components'; function EditorSettings() { @@ -40,7 +39,7 @@ function EditorSettings() { settingsApi: { fetchSettings, updateSettings }, }, state: { - settings: { googleAnalyticsId, publisherLogos }, + settings: { googleAnalyticsId }, }, } = useContext(ApiContext); @@ -71,10 +70,6 @@ function EditorSettings() { googleAnalyticsId={googleAnalyticsId} canManageSettings={canManageSettings} /> -
); diff --git a/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js b/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js index 8cb86e9a0726..cefbc8c2ca7b 100644 --- a/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js +++ b/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js @@ -31,6 +31,7 @@ describe('Settings View', () => { beforeEach(async () => { fixture = new Fixture(); + fixture.setFlags({ enableSettingsView: true }); await fixture.render(); diff --git a/assets/src/dashboard/components/pageStructure/index.js b/assets/src/dashboard/components/pageStructure/index.js index e03910455c85..91ded3b1d569 100644 --- a/assets/src/dashboard/components/pageStructure/index.js +++ b/assets/src/dashboard/components/pageStructure/index.js @@ -40,6 +40,7 @@ import { primaryPaths, secondaryPaths, Z_INDEX, + APP_ROUTES, } from '../../constants'; import useFocusOut from '../../utils/useFocusOut'; @@ -104,7 +105,9 @@ export function LeftRail() { const { newStoryURL, version } = useConfig(); const leftRailRef = useRef(null); const upperContentRef = useRef(null); + const enableInProgressViews = useFeature('enableInProgressViews'); + const enableSettingsViews = useFeature('enableSettingsView'); const { state: { sideBarVisible }, @@ -132,11 +135,17 @@ export function LeftRail() { }, [enableInProgressViews]); const enabledSecondaryPaths = useMemo(() => { + let copyOfSecondaryPaths = enableSettingsViews + ? [...secondaryPaths] + : secondaryPaths.filter( + (path) => !path.value.includes(APP_ROUTES.EDITOR_SETTINGS) + ); + if (enableInProgressViews) { - return secondaryPaths; + return copyOfSecondaryPaths; } - return secondaryPaths.filter((path) => !path.inProgress); - }, [enableInProgressViews]); + return copyOfSecondaryPaths.filter((path) => !path.inProgress); + }, [enableInProgressViews, enableSettingsViews]); const handleSideBarClose = useCallback(() => { if (sideBarVisible) { diff --git a/assets/src/dashboard/constants/index.js b/assets/src/dashboard/constants/index.js index 8dd93aea2a21..dfb7c6bdc323 100644 --- a/assets/src/dashboard/constants/index.js +++ b/assets/src/dashboard/constants/index.js @@ -85,7 +85,6 @@ export const secondaryPaths = [ { value: APP_ROUTES.EDITOR_SETTINGS, label: ROUTE_TITLES[APP_ROUTES.EDITOR_SETTINGS], - inProgress: true, }, { value: APP_ROUTES.SUPPORT, diff --git a/includes/Experiments.php b/includes/Experiments.php index ec37c247d2d0..e2cabbe812cd 100644 --- a/includes/Experiments.php +++ b/includes/Experiments.php @@ -229,6 +229,17 @@ public function get_experiments() { 'description' => __( 'Enable in-progress story actions', 'web-stories' ), 'group' => 'dashboard', ], + /** + * Author: @brittanyirl + * Issue: 3482 + * Creation date: 2020-08-10 + */ + [ + 'name' => 'enableSettingsView', + 'label' => __( 'Settings Views', 'web-stories' ), + 'description' => __( 'Enable settings view in dashboard', 'web-stories' ), + 'group' => 'dashboard', + ], /** * Author: @brittanyirl * Issue: 2381 From 3e79e1e747f2a2feef6665ae766717c3bb2974f5 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Mon, 10 Aug 2020 13:58:44 -0700 Subject: [PATCH 07/15] adding settings errors to toaster conditionally for enableSettingsView flag --- assets/src/dashboard/app/views/toaster/index.js | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/assets/src/dashboard/app/views/toaster/index.js b/assets/src/dashboard/app/views/toaster/index.js index afe5ce9a8faf..95fe569cec7c 100644 --- a/assets/src/dashboard/app/views/toaster/index.js +++ b/assets/src/dashboard/app/views/toaster/index.js @@ -18,6 +18,7 @@ * External dependencies */ import { useContext, useEffect } from 'react'; +import { useFeature } from 'flagged'; /** * Internal dependencies @@ -31,6 +32,7 @@ function ToasterView() { state: { stories: { error: storyError }, templates: { error: templateError }, + settings: { error: settingsError }, }, } = useContext(ApiContext); @@ -39,6 +41,8 @@ function ToasterView() { state: { activeToasts }, } = useToastContext(); + const enableSettingsView = useFeature('enableSettingsView'); + useEffect(() => { if (storyError?.id) { addToast({ @@ -59,6 +63,16 @@ function ToasterView() { } }, [templateError, addToast]); + useEffect(() => { + if (enableSettingsView && settingsError?.id) { + addToast({ + message: settingsError.message, + severity: ALERT_SEVERITY.ERROR, + id: settingsError.id, + }); + } + }, [settingsError, addToast, enableSettingsView]); + return ( Date: Mon, 10 Aug 2020 14:59:09 -0700 Subject: [PATCH 08/15] clean up for PR and forgotten karma test for errors. --- .../editorSettings/googleAnalytics/index.js | 4 ++-- .../googleAnalytics/stories/index.js | 4 ++-- .../karma/editorSettings.karma.js | 24 ++++++++++++++++++- .../editorSettings/test/editorSettings.js | 8 +++---- .../test/validateGoogleAnalyticsIdFormat.js | 17 ++++--------- 5 files changed, 35 insertions(+), 22 deletions(-) diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index e5c3f5c13727..45160e04a75e 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -45,6 +45,7 @@ export const TEXT = { SECTION_HEADING: __('Google Analytics Tracking ID', 'web-stories'), PLACEHOLDER: __('Enter your Google Analtyics Tracking ID', 'web-stories'), ARIA_LABEL: __('Enter your Google Analtyics Tracking ID', 'web-stories'), + INPUT_ERROR: __('Invalid ID format', 'web-stories'), }; function GoogleAnalyticsSettings({ @@ -61,12 +62,11 @@ function GoogleAnalyticsSettings({ const handleUpdateId = useCallback( (value) => { - // todo add validation to string format if (value.length === 0 || validateGoogleAnalyticsIdFormat(value)) { setInputError(''); return handleUpdateSettings({ newGoogleAnalyticsId: value }); } - return setInputError('Invalid ID format'); + return setInputError(TEXT.INPUT_ERROR); }, [handleUpdateSettings, setInputError] ); diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js index 059afb8a665a..68cae6a30ca3 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js @@ -34,7 +34,7 @@ export const _default = () => { return ( ); @@ -44,7 +44,7 @@ export const _CannotManageSettings = () => { return ( ); }; diff --git a/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js b/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js index cefbc8c2ca7b..2b374eedf52e 100644 --- a/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js +++ b/assets/src/dashboard/app/views/editorSettings/karma/editorSettings.karma.js @@ -91,5 +91,27 @@ describe('Settings View', () => { expect(newInput.value).toBe(googleAnalyticsId); }); - // it("it should not allow an update of google analytics id when id format doesn't match required format", () => {}); + it("it should not allow an update of google analytics id when id format doesn't match required format", async () => { + const settingsView = await fixture.screen.getByTestId('editor-settings'); + + const input = within(settingsView).getByRole('textbox'); + + await fixture.events.hover(input); + + await fixture.events.click(input); + + const inputLength = input.value.length; + + for (let iter = 0; iter < inputLength; iter++) { + // disable eslint to prevent overlapping .act calls + // eslint-disable-next-line no-await-in-loop + await fixture.events.keyboard.press('Backspace'); + } + + await fixture.events.keyboard.type('Clearly not a valid id'); + await fixture.events.keyboard.press('Enter'); + + const errorMessage = await fixture.screen.getByText('Invalid ID format'); + expect(errorMessage).toBeTruthy(); + }); }); diff --git a/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js index a801ff26ce77..34a3c55d7f69 100644 --- a/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js +++ b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js @@ -22,9 +22,10 @@ import PropTypes from 'prop-types'; * Internal dependencies */ import { renderWithTheme } from '../../../../testUtils'; -import EditorSettings from '../'; import { ApiContext } from '../../../api/apiProvider'; import { ConfigProvider } from '../../../config'; +import EditorSettings from '../'; +import { TEXT as GA_TEXT } from '../googleAnalytics'; const mockFetchSettings = jest.fn(); @@ -63,12 +64,9 @@ describe('Editor Settings: ', function () { ); - const googleAnalyticsHeading = getByText('Google Analytics Tracking ID'); + const googleAnalyticsHeading = getByText(GA_TEXT.SECTION_HEADING); expect(googleAnalyticsHeading).toBeInTheDocument(); - const publisherLogoHeading = getByText('Publisher Logo'); - expect(publisherLogoHeading).toBeInTheDocument(); - const input = getByRole('textbox'); expect(input).toBeDefined(); diff --git a/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js b/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js index 03c3902e06df..520f30d78e28 100644 --- a/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js +++ b/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js @@ -19,26 +19,19 @@ */ import { validateGoogleAnalyticsIdFormat } from '../'; -const validIdFormats = [ +const idsToValidate = [ ['UA-000000-56', true], ['ua-098765432-9875', true], -]; -const invalidIdFormats = [ ['78787878', false], ['ua--123448-0', false], ['clearly wrong', false], ]; describe('validateGoogleAnalyticsIdFormat', () => { - it.each(validIdFormats)('should return %s as true', (validId, expected) => { - const bool = validateGoogleAnalyticsIdFormat(validId); - expect(bool).toBe(expected); - }); - - it.each(invalidIdFormats)( - 'should return %s as false', - (invalidId, expected) => { - const bool = validateGoogleAnalyticsIdFormat(invalidId); + it.each(idsToValidate)( + 'should check if %s is valid %p %p', + (validId, expected) => { + const bool = validateGoogleAnalyticsIdFormat(validId); expect(bool).toBe(expected); } ); From e6ea1707977823e86430fee05c74444b2ba63ba8 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Mon, 10 Aug 2020 15:01:11 -0700 Subject: [PATCH 09/15] more test clean up --- .../app/views/editorSettings/test/editorSettings.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js index 34a3c55d7f69..efa0da0b56c2 100644 --- a/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js +++ b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js @@ -59,9 +59,9 @@ SettingsWrapper.propTypes = { }; describe('Editor Settings: ', function () { - it('should render settings page with google analytics and publisher logo sections', function () { + it('should render settings page with google analytics section', function () { const { getByText, getByRole } = renderWithTheme( - + ); const googleAnalyticsHeading = getByText(GA_TEXT.SECTION_HEADING); @@ -70,7 +70,7 @@ describe('Editor Settings: ', function () { const input = getByRole('textbox'); expect(input).toBeDefined(); - expect(input.value).toBe('123-45-98-not-an-id'); + expect(input.value).toBe('UA-098909-05'); expect(mockFetchSettings).toHaveBeenCalledTimes(1); }); From f97af66b441d666d4fd2a17f6a82b8f90fcc348c Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Mon, 10 Aug 2020 15:09:37 -0700 Subject: [PATCH 10/15] one more test clean up --- .../src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js b/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js index 520f30d78e28..534bf73072a1 100644 --- a/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js +++ b/assets/src/dashboard/utils/test/validateGoogleAnalyticsIdFormat.js @@ -29,7 +29,7 @@ const idsToValidate = [ describe('validateGoogleAnalyticsIdFormat', () => { it.each(idsToValidate)( - 'should check if %s is valid %p %p', + 'should take " %s " and return as %p google analytic id format', (validId, expected) => { const bool = validateGoogleAnalyticsIdFormat(validId); expect(bool).toBe(expected); From 577e753039e5b88f97941d138e58326dd7fe0167 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Tue, 11 Aug 2020 09:35:23 -0700 Subject: [PATCH 11/15] moving check for canManageSettings to app/index to pair with checking for enabled settings view flag. When both canManageSettins and that flag are true then the settings page is allowed to render. Doing same check in sideNav in app nav links to show or hide the settings page link. --- assets/src/dashboard/app/index.js | 6 ++-- .../editorSettings/googleAnalytics/index.js | 3 -- .../googleAnalytics/stories/index.js | 10 ------ .../googleAnalytics/test/googleAnalytics.js | 2 -- .../app/views/editorSettings/index.js | 4 --- .../editorSettings/test/editorSettings.js | 36 +++++++++---------- .../components/pageStructure/index.js | 9 +++-- 7 files changed, 27 insertions(+), 43 deletions(-) diff --git a/assets/src/dashboard/app/index.js b/assets/src/dashboard/app/index.js index 10e32b0ddb8c..bc7306448f88 100644 --- a/assets/src/dashboard/app/index.js +++ b/assets/src/dashboard/app/index.js @@ -49,7 +49,7 @@ import { } from '../components'; import ApiProvider from './api/apiProvider'; import { Route, RouterProvider, RouterContext, matchPath } from './router'; -import { ConfigProvider } from './config'; +import { ConfigProvider, useConfig } from './config'; import { EditorSettingsView, ExploreTemplatesView, @@ -65,7 +65,9 @@ const AppContent = () => { state: { currentPath }, } = useContext(RouterContext); - const enableSettingsView = useFeature('enableSettingsView'); + const { capabilities: { canManageSettings } = {} } = useConfig(); + const enableSettingsView = + useFeature('enableSettingsView') && canManageSettings; useEffect(() => { const dynamicPageTitle = ROUTE_TITLES[currentPath] || ROUTE_TITLES.DEFAULT; diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index 45160e04a75e..71b0eb3c0a96 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -51,7 +51,6 @@ export const TEXT = { function GoogleAnalyticsSettings({ googleAnalyticsId = '', handleUpdateSettings, - canManageSettings, }) { const [analyticsId, setAnalyticsId] = useState(googleAnalyticsId); const [inputError, setInputError] = useState(''); @@ -84,7 +83,6 @@ function GoogleAnalyticsSettings({ onEditCancel={handleCancelUpdateId} onEditComplete={handleUpdateId} placeholder={TEXT.PLACEHOLDER} - disabled={!canManageSettings} error={inputError} /> {TEXT.CONTEXT} @@ -94,7 +92,6 @@ function GoogleAnalyticsSettings({ } GoogleAnalyticsSettings.propTypes = { handleUpdateSettings: PropTypes.func, - canManageSettings: PropTypes.bool, googleAnalyticsId: PropTypes.string, }; diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js index 68cae6a30ca3..96430de7c923 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/stories/index.js @@ -31,16 +31,6 @@ export default { }; export const _default = () => { - return ( - - ); -}; - -export const _CannotManageSettings = () => { return ( ', function () { ); @@ -48,7 +47,6 @@ describe('Editor Settings: Google Analytics ', function () { ); diff --git a/assets/src/dashboard/app/views/editorSettings/index.js b/assets/src/dashboard/app/views/editorSettings/index.js index 1048ce4e896a..a87237b66183 100644 --- a/assets/src/dashboard/app/views/editorSettings/index.js +++ b/assets/src/dashboard/app/views/editorSettings/index.js @@ -27,13 +27,10 @@ import { useCallback, useContext, useEffect } from 'react'; * Internal dependencies */ import { ApiContext } from '../../api/apiProvider'; -import { useConfig } from '../../config'; import GoogleAnalyticsSettings from './googleAnalytics'; import { Wrapper, Header, Heading, Main } from './components'; function EditorSettings() { - const { capabilities: { canManageSettings } = {} } = useConfig(); - const { actions: { settingsApi: { fetchSettings, updateSettings }, @@ -68,7 +65,6 @@ function EditorSettings() { diff --git a/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js index efa0da0b56c2..d306d4a5d5aa 100644 --- a/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js +++ b/assets/src/dashboard/app/views/editorSettings/test/editorSettings.js @@ -23,39 +23,35 @@ import PropTypes from 'prop-types'; */ import { renderWithTheme } from '../../../../testUtils'; import { ApiContext } from '../../../api/apiProvider'; -import { ConfigProvider } from '../../../config'; import EditorSettings from '../'; import { TEXT as GA_TEXT } from '../googleAnalytics'; const mockFetchSettings = jest.fn(); -const SettingsWrapper = ({ canManageSettings, googleAnalyticsId }) => { +const SettingsWrapper = ({ googleAnalyticsId }) => { return ( - - - - - + }, + }} + > + + ); }; SettingsWrapper.propTypes = { googleAnalyticsId: PropTypes.string, - canManageSettings: PropTypes.bool, }; describe('Editor Settings: ', function () { diff --git a/assets/src/dashboard/components/pageStructure/index.js b/assets/src/dashboard/components/pageStructure/index.js index 91ded3b1d569..6151464e82ea 100644 --- a/assets/src/dashboard/components/pageStructure/index.js +++ b/assets/src/dashboard/components/pageStructure/index.js @@ -102,12 +102,17 @@ export const LeftRailContainer = styled.nav.attrs({ export function LeftRail() { const { state } = useRouteHistory(); - const { newStoryURL, version } = useConfig(); + const { + newStoryURL, + version, + capabilities: { canManageSettings } = {}, + } = useConfig(); const leftRailRef = useRef(null); const upperContentRef = useRef(null); const enableInProgressViews = useFeature('enableInProgressViews'); - const enableSettingsViews = useFeature('enableSettingsView'); + const enableSettingsViews = + useFeature('enableSettingsView') && canManageSettings; const { state: { sideBarVisible }, From 805a6e1bee571342730dc332371ea871dff01741 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Tue, 11 Aug 2020 09:52:10 -0700 Subject: [PATCH 12/15] update helper text to have link and better formatting for screen reader --- .../app/views/editorSettings/components.js | 5 +++++ .../editorSettings/googleAnalytics/index.js | 16 +++++++++++++--- 2 files changed, 18 insertions(+), 3 deletions(-) diff --git a/assets/src/dashboard/app/views/editorSettings/components.js b/assets/src/dashboard/app/views/editorSettings/components.js index 1ae996cfa035..bd0c5008a7f3 100644 --- a/assets/src/dashboard/app/views/editorSettings/components.js +++ b/assets/src/dashboard/app/views/editorSettings/components.js @@ -24,6 +24,7 @@ import styled from 'styled-components'; */ import { TypographyPresets } from '../../../components'; import { visuallyHiddenStyles } from '../../../utils/visuallyHiddenStyles'; +import { Link } from '../../../components/link'; export const Wrapper = styled.div` margin: 0 107px; @@ -73,6 +74,10 @@ export const FormContainer = styled.div` } `; +export const InlineLink = styled(Link)` + margin-left: 0.25em; +`; + export const HelperText = styled.p` ${TypographyPresets.Small}; color: ${({ theme }) => theme.colors.gray200}; diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index 71b0eb3c0a96..b7e3cd4ee80d 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -32,6 +32,7 @@ import { validateGoogleAnalyticsIdFormat } from '../../../../utils'; import { InlineInputForm } from '../../../../components'; import { FormContainer, + InlineLink, SettingForm, SettingHeading, TextInputHelperText, @@ -39,9 +40,12 @@ import { export const TEXT = { CONTEXT: __( - "The story editor will append a default, configurable AMP analytics configuration to your story. If you're interested in going beyond what the default configuration is, read this article.", + "The story editor will append a default, configurable AMP analytics configuration to your story. If you're interested in going beyond what the default configuration is, read this article:", 'web-stories' - ), // TODO update this text to have link to article once confirmed what article is + ), + CONTEXT_ARTICLE_LINK: + 'https://blog.amp.dev/2019/08/28/analytics-for-your-amp-stories/', + CONTEXT_ARTICLE: __('Analytics for your Web Stories', 'web-stories'), SECTION_HEADING: __('Google Analytics Tracking ID', 'web-stories'), PLACEHOLDER: __('Enter your Google Analtyics Tracking ID', 'web-stories'), ARIA_LABEL: __('Enter your Google Analtyics Tracking ID', 'web-stories'), @@ -85,7 +89,13 @@ function GoogleAnalyticsSettings({ placeholder={TEXT.PLACEHOLDER} error={inputError} /> - {TEXT.CONTEXT} + + {TEXT.CONTEXT} + + {TEXT.CONTEXT_ARTICLE} + + {'.'} + ); From d91058f7bbbc441e74264dea64f379ec6b7e8e0f Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Tue, 11 Aug 2020 10:02:39 -0700 Subject: [PATCH 13/15] gut disabled prop from input as we do not need this here and i had only added it to take care of canmanageSettings --- assets/src/dashboard/components/inlineInputForm/index.js | 3 --- 1 file changed, 3 deletions(-) diff --git a/assets/src/dashboard/components/inlineInputForm/index.js b/assets/src/dashboard/components/inlineInputForm/index.js index 4d37f28c2851..df6437f10107 100644 --- a/assets/src/dashboard/components/inlineInputForm/index.js +++ b/assets/src/dashboard/components/inlineInputForm/index.js @@ -45,7 +45,6 @@ const InlineInputForm = ({ onEditComplete, placeholder, value, - disabled, }) => { const inputContainerRef = useRef(null); const [newValue, setNewValue] = useState(value); @@ -89,7 +88,6 @@ const InlineInputForm = ({ onKeyDown={handleKeyPress} onChange={handleChange} placeholder={placeholder} - disabled={disabled} error={error} /> {error && {error}} @@ -98,7 +96,6 @@ const InlineInputForm = ({ }; InlineInputForm.propTypes = { - disabled: PropTypes.bool, error: PropTypes.string, id: PropTypes.oneOfType([PropTypes.string, PropTypes.number]).isRequired, label: PropTypes.string.isRequired, From 2d7fdf08439ebe2bb6969e083f91b1ca09c294ac Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Tue, 11 Aug 2020 10:46:11 -0700 Subject: [PATCH 14/15] move regex pattern for validation above function --- assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js b/assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js index 081ed335df18..5f7abef2c9d1 100644 --- a/assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js +++ b/assets/src/dashboard/utils/validateGoogleAnalyticsIdFormat.js @@ -14,7 +14,8 @@ * limitations under the License. */ +const googleAnalyticsIdFormatRegex = /^ua-\d+-\d+$/; + export default function validateGoogleAnalyticsIdFormat(value = '') { - const googleAnalyticsIdFormatRegex = /^ua-\d+-\d+$/; return Boolean(value.toLowerCase().match(googleAnalyticsIdFormatRegex)); } From ba7794f7c4e2dfe3f3a41f233b3f4c8c4dcc14e7 Mon Sep 17 00:00:00 2001 From: Brittany Feenstra Date: Tue, 11 Aug 2020 11:48:13 -0700 Subject: [PATCH 15/15] no more period after article link in helper text --- .../dashboard/app/views/editorSettings/googleAnalytics/index.js | 1 - 1 file changed, 1 deletion(-) diff --git a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js index b7e3cd4ee80d..bb986330c8ac 100644 --- a/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js +++ b/assets/src/dashboard/app/views/editorSettings/googleAnalytics/index.js @@ -94,7 +94,6 @@ function GoogleAnalyticsSettings({ {TEXT.CONTEXT_ARTICLE} - {'.'}