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

Use EditableInput component in Toolbar #2238

Merged
merged 7 commits into from
Aug 15, 2023

Conversation

lindapaiste
Copy link
Collaborator

Fixes #2237
(I can put in a PR with a 3-line CSS change if we want to fix just that issue and not all this other stuff)

Background:
We have a nice EditableInput UI component which was created by @andrewn for the collection details page in #1164. This component displays a text with a pencil icon and converts it to an input when clicked. We use this same behavior when displaying the project name in the editor toolbar but we aren't using this component for it. We should use it!

Changes:

  • EditableInput component:
    • Can override the aria-label via props (so we can still use t('Toolbar.EditSketchARIA')).
    • Can pass a prop disabled to render as plain text with no pencil icon and no hover effects (issue Project name looks clickable when viewing others' projects #2237)
    • Added support for canceling by pressing the Esc key. This will discard any text typed into the input and revert to the initial value, without submitting or saving anything.
    • Sets an aria-hidden property on the label and the input when they are hidden via CSS display: none
  • Toolbar component:
    • Use the EditableInput component.
    • Delete a bunch of code which duplicates what's already handed by the EditableInput.
    • Delete most of the CSS for the .toolbar__project-name, but leave behind a few overrides which apply to the EditableInput when used inside the .toolbar to keep the style the same as the current (thinner border, uses 'secondary-text-color' instead of 'inactive-text-color').
  • isEditing state management:
    • isEditing is now a local component state of the EditableInput and does not need to be in Redux.
    • Delete Redux actions showEditProjectName and hideEditProjectName and remove isEditingName from state.
    • The Toolbar.unit.test.jsx file can no longer see if these actions have been dispatched, so instead I'm looking at the disabled and focused states of the <input> element to determine whether or not it is editing.

I have verified that this pull request:

  • has no linting errors (npm run lint)
  • has no test errors (npm run test)
  • is from a uniquely-named feature branch and is up to date with the develop branch.
  • is descriptively named and links to an issue number, i.e. Fixes #123

@lindapaiste lindapaiste requested a review from raclim June 10, 2023 20:10
# Conflicts:
#	client/modules/IDE/components/Header/Toolbar.jsx
# Conflicts:
#	client/modules/IDE/components/Header/Toolbar.jsx
#	client/modules/IDE/components/Header/Toolbar.unit.test.jsx
@lindapaiste
Copy link
Collaborator Author

@dewanshDT I merged your Toolbar changes into this. Now see if you can merge this branch into your MobileNav and use the ProjectName component when displaying the name in the nav bar so that we have the ability to edit project names on mobile.

@dewanshDT
Copy link
Collaborator

Okay

@lindapaiste lindapaiste merged commit 77fd047 into processing:develop Aug 15, 2023
1 check passed
@lindapaiste lindapaiste deleted the fix/toolbar-title branch August 15, 2023 19:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Project name looks clickable when viewing others' projects
2 participants