Preview comments navigation#2440
Merged
Merged
Conversation
The commenting Popover called useFloating unconditionally, so every section and content element set up floating-positioning state to back a thread-list popover that is only ever shown while selected - and a badge that renders nothing at all when there are no unresolved threads. Move useFloating and the floating thread list into a child component that mounts only while the popover is selected. The badge stays always-mounted and serves as the floating reference; outside-click and Escape dismissal move into the open child alongside the floating state they depend on.
Add a CommentDisplayFilterProvider holding a resolution state (unresolved or all) and a segmented Unresolved/All control in the floating toolbar to switch between them. The commenting section and content element decorators, the preview popovers, and the editable text highlights read the filter instead of always showing only unresolved threads, so it reveals or hides resolved comments across the whole entry. ThreadsBadge gains an optional resolution prop defaulting to unresolved, leaving the inline editing badges unchanged.
Add a CommentNavigationProvider that derives the located comment threads matching the current display filter in document order and exposes a count together with goToNext and goToPrevious actions. The floating toolbar shows the count and up/down arrows to cycle through the comments, wrapping around at the ends. Navigation is published through an emitter: each preview popover registers as the target for its subject and, when navigated to, scrolls into view and opens. Targets carry their section and excursion location so a later change can activate the right excursion before scrolling.
Move the comment navigation emitter and its subscriber hooks into a core module so Content can listen without pulling the lazily loaded commenting code into the main bundle. When navigation targets a thread in an excursion, Content activates that excursion; when it targets a main storyline thread, it returns from the current excursion. The emitter now replays the last navigation to a target that subscribes only afterwards, so the popover that mounts once its excursion is activated still scrolls into view and opens.
Track the current index in the navigation engine and surface it as a one-based position so the toolbar can show the current comment while stepping. When navigating to a thread, pass its id to the open popover's thread list as the highlighted thread, so the current comment stands out (and scrolls into view within the list) when an element has several. Keep the popover from blinking while stepping within one element: only open and scroll when it is not already open, and ignore presses on the comment toolbar in the popover's outside-click dismissal. Couple selection with navigation: selecting a subject (e.g. by clicking its badge) moves the cursor onto that subject's first thread, so the position indicator reflects it and the arrows continue from there.
Arrange the floating comment toolbar as a single dark bar: the
position indicator ("– /") first, then the resolution filter as a
segmented Unresolved/All control showing each filter's comment count,
then the up/down navigation arrows, then the add-comment button as a
prominent accent action. The position indicator omits the total since
the active segment's count already conveys it. Controls sit
transparently on the shared dark (primary color) bar; the active
filter segment reads as a lighter pill with a count chip.
Segment counts come from the located comment threads; the count badges
are aria-hidden so the segments keep their plain accessible names.
Add an expandResolved prop to ThreadList. When set, the resolved section is expanded by default instead of collapsed behind the count pill, and when every thread of the subject is resolved the resolved comments are shown rather than auto-opening the new thread form. The manual toggle and explicit showNewForm still take precedence. The commenting popover sets expandResolved when the resolution filter is 'all', so switching the toolbar filter to all reveals resolved comments in place. Editor usages omit the prop and keep their previous behavior.
Comment thread navigation in the preview now steps through threads in the same position-based order the editor's comments view already uses, rather than an arbitrary one. The logic that orders threads within a content element lived in the editor API and depended on Slate, so the frontend could not reuse it without pulling Slate into the always-on bundle. A shared review registry now provides this per content element type to both the editor and the frontend commenting mode, with the Slate-based comparison loaded only as part of the commenting code.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
REDMINE-21261