From fc6496cee8c8204492bf76df5452e8005df2293f Mon Sep 17 00:00:00 2001 From: Linda Paiste Date: Sat, 10 Jun 2023 14:34:13 -0500 Subject: [PATCH 1/2] use EditableInput in Toolbar --- client/constants.js | 2 - client/modules/IDE/actions/project.js | 12 --- .../modules/IDE/components/EditableInput.jsx | 36 ++++++-- client/modules/IDE/components/Toolbar.jsx | 87 +++---------------- .../IDE/components/Toolbar.unit.test.jsx | 16 ++-- client/modules/IDE/reducers/project.js | 4 - client/styles/components/_editable-input.scss | 8 ++ client/styles/components/_toolbar.scss | 39 ++------- 8 files changed, 64 insertions(+), 140 deletions(-) diff --git a/client/constants.js b/client/constants.js index ec0e4107ac..565d716fa6 100644 --- a/client/constants.js +++ b/client/constants.js @@ -30,8 +30,6 @@ export const PROJECT_SAVE_SUCCESS = 'PROJECT_SAVE_SUCCESS'; export const PROJECT_SAVE_FAIL = 'PROJECT_SAVE_FAIL'; export const NEW_PROJECT = 'NEW_PROJECT'; export const RESET_PROJECT = 'RESET_PROJECT'; -export const SHOW_EDIT_PROJECT_NAME = 'SHOW_EDIT_PROJECT_NAME'; -export const HIDE_EDIT_PROJECT_NAME = 'HIDE_EDIT_PROJECT_NAME'; export const SET_PROJECT = 'SET_PROJECT'; export const SET_PROJECTS = 'SET_PROJECTS'; diff --git a/client/modules/IDE/actions/project.js b/client/modules/IDE/actions/project.js index 9a528a34f6..3709d07a2e 100644 --- a/client/modules/IDE/actions/project.js +++ b/client/modules/IDE/actions/project.js @@ -351,18 +351,6 @@ export function cloneProject(project) { }; } -export function showEditProjectName() { - return { - type: ActionTypes.SHOW_EDIT_PROJECT_NAME - }; -} - -export function hideEditProjectName() { - return { - type: ActionTypes.HIDE_EDIT_PROJECT_NAME - }; -} - export function setProjectSavedTime(updatedAt) { return { type: ActionTypes.SET_PROJECT_SAVED_TIME, diff --git a/client/modules/IDE/components/EditableInput.jsx b/client/modules/IDE/components/EditableInput.jsx index 871c6470a5..405cc61920 100644 --- a/client/modules/IDE/components/EditableInput.jsx +++ b/client/modules/IDE/components/EditableInput.jsx @@ -11,7 +11,9 @@ function EditableInput({ emptyPlaceholder, InputComponent, inputProps, - onChange + onChange, + disabled, + 'aria-label': ariaLabel }) { const [isEditing, setIsEditing] = React.useState(false); const [currentValue, setCurrentValue] = React.useState(value || ''); @@ -19,12 +21,14 @@ function EditableInput({ const hasValue = currentValue !== ''; const classes = `editable-input editable-input--${ isEditing ? 'is-editing' : 'is-not-editing' - } editable-input--${hasValue ? 'has-value' : 'has-placeholder'}`; - const inputRef = React.createRef(); + } editable-input--${hasValue ? 'has-value' : 'has-placeholder'} ${ + disabled ? 'editable-input--disabled' : '' + }`; + const inputRef = React.useRef(); const { t } = useTranslation(); React.useEffect(() => { if (isEditing) { - inputRef.current.focus(); + inputRef.current?.focus(); } }, [isEditing]); @@ -32,6 +36,11 @@ function EditableInput({ setIsEditing(true); } + function cancelEditing() { + setIsEditing(false); + setCurrentValue(value); + } + function doneEditing() { setIsEditing(false); @@ -51,6 +60,8 @@ function EditableInput({ function checkForKeyAction(event) { if (event.key === 'Enter') { doneEditing(); + } else if (event.key === 'Escape' || event.key === 'Esc') { + cancelEditing(); } } @@ -59,7 +70,11 @@ function EditableInput({ - { - this.projectNameInput = element; + inputProps={{ + maxLength: 128, + 'aria-label': this.props.t('Toolbar.NewSketchNameARIA') }} - onBlur={this.handleProjectNameSave} - onKeyPress={this.handleKeyPress} + validate={(text) => text.trim().length > 0} + onChange={this.handleProjectNameSave} /> {(() => { if (this.props.owner) { @@ -205,11 +149,8 @@ Toolbar.propTypes = { }), project: PropTypes.shape({ name: PropTypes.string.isRequired, - isEditingName: PropTypes.bool, id: PropTypes.string }).isRequired, - showEditProjectName: PropTypes.func.isRequired, - hideEditProjectName: PropTypes.func.isRequired, infiniteLoop: PropTypes.bool.isRequired, autorefresh: PropTypes.bool.isRequired, setAutorefresh: PropTypes.func.isRequired, diff --git a/client/modules/IDE/components/Toolbar.unit.test.jsx b/client/modules/IDE/components/Toolbar.unit.test.jsx index d5e83dd69d..8ce45aaaaf 100644 --- a/client/modules/IDE/components/Toolbar.unit.test.jsx +++ b/client/modules/IDE/components/Toolbar.unit.test.jsx @@ -31,7 +31,6 @@ const renderComponent = (extraProps = {}) => { }, project: { name: 'testname', - isEditingName: false, id: 'id' }, t: jest.fn() @@ -46,28 +45,31 @@ const renderComponent = (extraProps = {}) => { describe('', () => { it('sketch owner can switch to sketch name editing mode', async () => { - const props = renderComponent(); + renderComponent(); const sketchName = screen.getByLabelText('Edit sketch name'); fireEvent.click(sketchName); - await waitFor(() => expect(props.showEditProjectName).toHaveBeenCalled()); + await waitFor(() => { + expect(screen.getByLabelText('New sketch name')).toHaveFocus(); + expect(screen.getByLabelText('New sketch name')).toBeEnabled(); + }); }); it("non-owner can't switch to sketch editing mode", async () => { - const props = renderComponent({ currentUser: 'not-me' }); + renderComponent({ currentUser: 'not-me' }); const sketchName = screen.getByLabelText('Edit sketch name'); fireEvent.click(sketchName); expect(sketchName).toBeDisabled(); await waitFor(() => - expect(props.showEditProjectName).not.toHaveBeenCalled() + expect(screen.getByLabelText('New sketch name')).toBeDisabled() ); }); it('sketch owner can change name', async () => { - const props = renderComponent({ project: { isEditingName: true } }); + const props = renderComponent(); const sketchNameInput = screen.getByLabelText('New sketch name'); fireEvent.change(sketchNameInput, { @@ -82,7 +84,7 @@ describe('', () => { }); it("sketch owner can't change to empty name", async () => { - const props = renderComponent({ project: { isEditingName: true } }); + const props = renderComponent(); const sketchNameInput = screen.getByLabelText('New sketch name'); fireEvent.change(sketchNameInput, { target: { value: '' } }); diff --git a/client/modules/IDE/reducers/project.js b/client/modules/IDE/reducers/project.js index df0da82411..89a03529e6 100644 --- a/client/modules/IDE/reducers/project.js +++ b/client/modules/IDE/reducers/project.js @@ -37,10 +37,6 @@ const project = (state, action) => { }; case ActionTypes.RESET_PROJECT: return initialState(); - case ActionTypes.SHOW_EDIT_PROJECT_NAME: - return Object.assign({}, state, { isEditingName: true }); - case ActionTypes.HIDE_EDIT_PROJECT_NAME: - return Object.assign({}, state, { isEditingName: false }); case ActionTypes.SET_PROJECT_SAVED_TIME: return Object.assign({}, state, { updatedAt: action.value }); case ActionTypes.START_SAVING_PROJECT: diff --git a/client/styles/components/_editable-input.scss b/client/styles/components/_editable-input.scss index f750af0d8a..07e157b613 100644 --- a/client/styles/components/_editable-input.scss +++ b/client/styles/components/_editable-input.scss @@ -6,6 +6,7 @@ button.editable-input__label { display: flex; + align-items: center; @include themify() { color: getThemifyVariable('inactive-text-color'); @@ -35,6 +36,13 @@ button.editable-input__label { height: 1.5rem; } +.editable-input--disabled { + pointer-events: none; + .editable-input__icon { + display: none; + } +} + .editable-input--is-not-editing .editable-input__input, .editable-input--is-editing .editable-input__label { display: none; diff --git a/client/styles/components/_toolbar.scss b/client/styles/components/_toolbar.scss index cd74ec8f6f..e4b9b86742 100644 --- a/client/styles/components/_toolbar.scss +++ b/client/styles/components/_toolbar.scss @@ -98,40 +98,23 @@ } .toolbar__project-name-container { - @include themify() { - border-color: getThemifyVariable('inactive-text-color'); - } margin-left: #{10 / $base-font-size}rem; padding-left: #{10 / $base-font-size}rem; display: flex; align-items: center; } -.toolbar__project-name { +.toolbar .editable-input__label { @include themify() { color: getThemifyVariable('secondary-text-color'); - &:hover { - color: getThemifyVariable('logo-color'); - & .toolbar__edit-name-button path { - fill: getThemifyVariable('logo-color'); - } + & path { + fill: getThemifyVariable('secondary-text-color'); } } - cursor: pointer; - display: flex; - align-items: center; - - .toolbar__project-name-container--editing & { - display: none; - } } -.toolbar__project-name-input { - display: none; - border: 0px; - .toolbar__project-name-container--editing & { - display: block; - } +.toolbar .editable-input__input { + border: 0; } .toolbar__project-owner { @@ -160,15 +143,3 @@ color:getThemifyVariable('logo-color'); } } - -.toolbar__edit-name-button { - display: inline-block; - vertical-align: top; - width: #{18 / $base-font-size}rem; - height: #{18 / $base-font-size}rem; - @include themify() { - & path { - fill: getThemifyVariable('secondary-text-color'); - } - } -} From a49f83418fdc60a268846afb38661e1ebc689d1a Mon Sep 17 00:00:00 2001 From: Linda Paiste Date: Sun, 13 Aug 2023 17:12:54 -0500 Subject: [PATCH 2/2] move ProjectName to a separate file --- .../IDE/components/Header/ProjectName.jsx | 28 +++++++++++++++++++ .../modules/IDE/components/Header/Toolbar.jsx | 16 ++--------- 2 files changed, 30 insertions(+), 14 deletions(-) create mode 100644 client/modules/IDE/components/Header/ProjectName.jsx diff --git a/client/modules/IDE/components/Header/ProjectName.jsx b/client/modules/IDE/components/Header/ProjectName.jsx new file mode 100644 index 0000000000..489a2105ac --- /dev/null +++ b/client/modules/IDE/components/Header/ProjectName.jsx @@ -0,0 +1,28 @@ +import React from 'react'; +import { useTranslation } from 'react-i18next'; +import { useSelector } from 'react-redux'; +import { useSketchActions } from '../../hooks'; +import { selectProjectName } from '../../selectors/project'; +import EditableInput from '../EditableInput'; + +export default function ProjectName() { + const { t } = useTranslation(); + + const { changeSketchName, canEditProjectName } = useSketchActions(); + + const projectName = useSelector(selectProjectName); + + return ( + text.trim().length > 0} + onChange={changeSketchName} + /> + ); +} diff --git a/client/modules/IDE/components/Header/Toolbar.jsx b/client/modules/IDE/components/Header/Toolbar.jsx index eb0689e303..dc3731cec6 100644 --- a/client/modules/IDE/components/Header/Toolbar.jsx +++ b/client/modules/IDE/components/Header/Toolbar.jsx @@ -15,12 +15,11 @@ import { setGridOutput, setTextOutput } from '../../actions/preferences'; -import { useSketchActions } from '../../hooks'; import PlayIcon from '../../../../images/play.svg'; import StopIcon from '../../../../images/stop.svg'; import PreferencesIcon from '../../../../images/preferences.svg'; -import EditableInput from '../EditableInput'; +import ProjectName from './ProjectName'; const Toolbar = (props) => { const { isPlaying, infiniteLoop, preferencesIsVisible } = useSelector( @@ -31,7 +30,6 @@ const Toolbar = (props) => { const dispatch = useDispatch(); const { t } = useTranslation(); - const { changeSketchName, canEditProjectName } = useSketchActions(); const playButtonClass = classNames({ 'toolbar__play-button': true, @@ -96,17 +94,7 @@ const Toolbar = (props) => {
- text.trim().length > 0} - onChange={changeSketchName} - /> + {(() => { if (project.owner) { return (