Conversation
Improved aria labels in UndoRedoButtons to use the localized string Added ButtonGroup (and conditional Separators based on variant)
…w used in Footnote Editor and Comment Editor) Added optional canUndo and canRedo props to EditorKeyboardShortcuts component to prevent keyboard shortcuts use in cases where undo or redo might do something in the editor even though they should not (see PT-3922).
# Conflicts: # lib/platform-bible-react/dist/index.cjs # lib/platform-bible-react/dist/index.cjs.map # lib/platform-bible-react/dist/index.js # lib/platform-bible-react/dist/index.js.map
tombogle
requested review from
irahopkinson,
jolierabideau,
lyonsil,
rolfheij-sil and
tjcouch-sil
as code owners
|
Introduces a trivial(?) regression in that the tooltip for the Comment editor now says "Save" instead of "Save Comment". That could be fixed if desired, but I wasn't very sure if that was intentional/necessary, so I didn't want to do additional work to "fix" it unless it is actually desirable. |
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
| <CancelAcceptButtons | ||
| onCancelClick={onClose} | ||
| onAcceptClick={closeAndSave} | ||
| canAccept={!isAtInitialState} | ||
| localizedStrings={localizedStrings} | ||
| /> |
There was a problem hiding this comment.
🔴 FootnoteEditor accept button disabled when only custom caller text changes
The canAccept condition !isAtInitialState || originalCallerType !== callerType does not account for changes to the custom caller text. When a note already has a custom caller (e.g. *) and the user changes it to a different value (e.g. +), both originalCallerType and callerType remain 'custom', and isAtInitialState remains true (because the editor USJ content is unchanged). This means canAccept evaluates to false, the accept button is disabled, and the user cannot commit the change to the parent editor via closeAndSave (which is the only path that calls replaceEmbedUpdate with applyToParent=true — see footnote-editor.component.tsx:270-296). The cancel button calls onClose directly, which discards the caller change.
Was this helpful? React with 👍 or 👎 to provide feedback.
lib/platform-bible-react/src/components/advanced/comment-editor/comment-editor.component.tsx
Show resolved
Hide resolved
|
|
Removed useless canCancel and variant props from CancelAcceptButtons component Added stories for CancelAcceptButtons component Addressed the problem of not being able to save a footnote change if the only thing the user did was to change the caller type.
|
The copy button in |
merchako
left a comment
merchako
left a comment
There was a problem hiding this comment.
@merchako reviewed 5 files and made 1 comment.
Reviewable status: 5 of 19 files reviewed, 1 unresolved discussion (waiting on tombogle).
lib/platform-bible-react/src/components/advanced/footnote-editor/footnote-editor.component.tsx line 523 at r3 (raw file):
Can we make this a <ButtonGroup> parents of the button groups <UndoRedoButtons> and <CancelAcceptButtons>?
Each component internally uses
ButtonGroup. But the two groups are not nested in a shared parentButtonGroup— they sit in a flexdivwithtw-gap-4. Alex's requested CSS gap logic (has-[>[data-slot=button-group]]:tw-gap-2) won't apply.
2 participants
This change is