Core 20 highlight edit navigation #2558
Conversation
RoyEJohnson
force-pushed
the
core-20-highlight-edit-navigation
branch
from
f4806f7 to
4a38c4a
Compare
RoyEJohnson
force-pushed
the
core-20-highlight-edit-navigation
branch
from
1683277 to
9f6495c
Compare
RoyEJohnson
force-pushed
the
core-20-highlight-edit-navigation
branch
from
65e322d to
51bd861
Compare
RoyEJohnson
force-pushed
the
core-20-highlight-edit-navigation
branch
from
51bd861 to
600cdff
Compare
| display: flex; | ||
| align-items: center; | ||
| margin: 0; | ||
| padding: ${modalPadding * 0.5}rem 0; |
There was a problem hiding this comment.
Padding was doubled in the dialog title bar
| .highlight { | ||
| position: relative; | ||
| z-index: 1; | ||
| user-select: none; |
There was a problem hiding this comment.
Prevent text selection in highlights, in particular by double-clicking, but overlapping highlights aren't allowed anyway, so this prevents error messages.
| // Styling is expensive and most Cards don't need to render | ||
| function PreCard(props: CardProps) { | ||
| const computedProps = useComputedProps(props); | ||
| const hideUnfocusedEditCard = computedProps.annotation ? {} : {isHidden: !props.shouldFocusCard}; |
There was a problem hiding this comment.
Edit card now only appears when making edits to the highlight/note
| return [cardsHeights, onHeightChange] as const; | ||
| } | ||
|
|
||
| function rangeString({startOffset, endOffset}: Highlight['range']) { |
There was a problem hiding this comment.
Functional changes are here through the end of useFocusedHighlight. There's a section below where most of the code was extracted to make the function, and replaced with a call to useFocusedHighlight. It's more obvious what I did if you look at the individual commit.
|
|
||
| function useSaveAnnotation( | ||
| props: EditCardProps, | ||
| { data, pageId, locationFilterId, highlight, onCancel }: EditCardProps, |
There was a problem hiding this comment.
Just a bit of revising to props handling
| loggedOut: boolean; | ||
| } | ||
|
|
||
| function useCloseAndTrack(closeFn: () => void) { |
There was a problem hiding this comment.
Some refactoring for clarity.
| export const useFocusLost = (ref: React.RefObject<HTMLElement>, isEnabled: boolean, cb: () => void) => { | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| React.useEffect(onFocusInOrOutHandler(ref, isEnabled, cb, 'focusout'), [ref, isEnabled, cb]); | ||
| React.useEffect(() => onFocusInOrOutHandler(ref, isEnabled, cb, 'focusout')(), [ref, isEnabled, cb]); |
There was a problem hiding this comment.
useEffect prefers to have an anonymous function, and this saves an eslint-disable comment
| } | ||
| const highlight = highlights.find(h => | ||
| h.elements && h.elements.some(el => | ||
| h.elements && (h.elements as Element[]).some(el => |
There was a problem hiding this comment.
This cast is better than using any below.
RoyEJohnson
force-pushed
the
core-20-highlight-edit-navigation
branch
from
ebc469f to
294aae1
Compare
|
@jivey I have modified the highlight editor to show instructions (only) when a highlight is focused, to make the editor more discoverable. |
RoyEJohnson
force-pushed
the
core-20-highlight-edit-navigation
branch
from
025bcab to
a34e3f3
Compare
|
@RoyEJohnson Something that just came to mind - if we always show the edit card while authenticated, is it odd that this message shows up? Because you can't edit it while unauthenticated? |
If not authenticated, show login button If no annotation, show instruction box If annotation or edit-highight sequence has happened, show edit card
RoyEJohnson
force-pushed
the
core-20-highlight-edit-navigation
branch
from
e537e1e to
f3f4794
Compare
|
I had goofed it up: it wasn't supposed to always show the edit card. It should show
|
CORE-20
CORE-459
The latter one describes using tab to get to the dialog, but we took a different approach (we want to be able to tab through the highlights without the detours, and use a different key to go into the editor).