{!submitting && (!thread || thread.messages.length === 0) && (
-
{t('no_comments')}
+
{t('no_comments')}
)}
{thread.messages.map(comment => (
@@ -143,9 +149,7 @@ function CommentEntry({
))}
{submitting && (
-
-
-
+
)}
{permissions.comment && (
@@ -163,7 +167,11 @@ function CommentEntry({
{permissions.comment && permissions.write && (
- {t('resolve')}
+ }
+ bs5={}
+ />
+ {t('resolve')}
)}
{permissions.comment && (
@@ -171,7 +179,11 @@ function CommentEntry({
onClick={handleOnReply}
disabled={!replyContent.length}
>
- {t('reply')}
+ }
+ bs5={}
+ />
+ {t('reply')}
)}
diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx
index 5a8ef217c8..689f532e69 100644
--- a/services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx
+++ b/services/web/frontend/js/features/source-editor/components/review-panel/entries/resolved-comment-entry.tsx
@@ -61,7 +61,7 @@ function ResolvedCommentEntry({
<>
diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/overview-container.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/overview-container.tsx
index d45d2eb8e8..cdfb0fa6f0 100644
--- a/services/web/frontend/js/features/source-editor/components/review-panel/overview-container.tsx
+++ b/services/web/frontend/js/features/source-editor/components/review-panel/overview-container.tsx
@@ -2,11 +2,11 @@ import Container from './container'
import Toggler from './toggler'
import Toolbar from './toolbar/toolbar'
import Nav from './nav'
-import Icon from '../../../../shared/components/icon'
import OverviewFile from './overview-file'
import { useReviewPanelValueContext } from '../../context/review-panel/review-panel-context'
import { useFileTreeData } from '@/shared/context/file-tree-data-context'
import { memo } from 'react'
+import LoadingSpinner from '@/shared/components/loading-spinner'
function OverviewContainer() {
const { isOverviewLoading } = useReviewPanelValueContext()
@@ -24,9 +24,7 @@ function OverviewContainer() {
aria-labelledby="review-panel-tab-overview"
>
{isOverviewLoading ? (
-
-
-
+
) : (
docs?.map(doc => (
-
+ }
+ bs5={}
+ />
{docPath}
{docCollapsed && (
diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx
index ccfd76f6a1..fe65f19675 100644
--- a/services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx
+++ b/services/web/frontend/js/features/source-editor/components/review-panel/positioned-entries.tsx
@@ -1,4 +1,4 @@
-import { useEffect, useRef } from 'react'
+import { useEffect, useLayoutEffect, useRef, useCallback } from 'react'
import { useLayoutContext } from '../../../../shared/context/layout-context'
import {
ReviewPanelEntry,
@@ -6,7 +6,6 @@ import {
} from '../../../../../../types/review-panel/entry'
import { debugConsole } from '../../../../utils/debugging'
import { useReviewPanelValueContext } from '../../context/review-panel/review-panel-context'
-import useEventListener from '../../../../shared/hooks/use-event-listener'
import { ReviewPanelDocEntries } from '../../../../../../types/review-panel/review-panel'
import { dispatchReviewPanelLayout } from '../../extensions/changes/change-manager'
import { isEqual } from 'lodash'
@@ -214,255 +213,269 @@ function PositionedEntries({
previousLayoutInfoRef.current = initialLayoutInfo
}
- const layout = (animate = true) => {
- const container = containerRef.current
- if (!container) {
- return
- }
-
- const padding = reviewPanelOpen ? 8 : 4
- const toolbarPaddedHeight = reviewPanelOpen ? toolbarHeight + 6 : 0
- const navPaddedHeight = reviewPanelOpen ? navHeight + 4 : 0
-
- // Create a list of entry views, typing together DOM elements and model.
- // No measuring or style change is done at this point.
- const entryViews: EntryView[] = []
-
- // TODO: Look into tying the entry to the DOM element without going via a DOM data attribute
- for (const wrapper of container.querySelectorAll(
- '.rp-entry-wrapper'
- )) {
- const entryId = wrapper.dataset.entryId as
- | EntryView['entryId']
- | undefined
- if (!entryId) {
- throw new Error('Could not find an entry ID')
+ const layout = useCallback(
+ (animate = true) => {
+ const container = containerRef.current
+ if (!container) {
+ return
}
- const entry = entries.find(value => value[0] === entryId)?.[1]
- if (!entry) {
- throw new Error(`Could not find an entry for ID ${entryId}`)
+ const padding = reviewPanelOpen ? 8 : 4
+ const toolbarPaddedHeight = reviewPanelOpen ? toolbarHeight + 6 : 0
+ const navPaddedHeight = reviewPanelOpen ? navHeight + 4 : 0
+
+ // Create a list of entry views, typing together DOM elements and model.
+ // No measuring or style change is done at this point.
+ const entryViews: EntryView[] = []
+
+ // TODO: Look into tying the entry to the DOM element without going via a DOM data attribute
+ for (const wrapper of container.querySelectorAll(
+ '.rp-entry-wrapper'
+ )) {
+ const entryId = wrapper.dataset.entryId as
+ | EntryView['entryId']
+ | undefined
+ if (!entryId) {
+ throw new Error('Could not find an entry ID')
+ }
+
+ const entry = entries.find(value => value[0] === entryId)?.[1]
+ if (!entry) {
+ throw new Error(`Could not find an entry for ID ${entryId}`)
+ }
+
+ const indicator = wrapper.querySelector(
+ '.rp-entry-indicator'
+ )
+ const box = wrapper.querySelector('.rp-entry')
+ const callout = wrapper.querySelector('.rp-entry-callout')
+ const layoutElement = reviewPanelOpen ? box : indicator
+
+ if (box && callout && layoutElement) {
+ const previousPositions =
+ previousLayoutInfoRef.current?.positions.find(
+ pos => pos.entryId === entryId
+ )?.positions
+ const hasScreenPos = Boolean(entry.screenPos)
+ entryViews.push({
+ entryId,
+ wrapper,
+ indicator,
+ box,
+ callout,
+ layout: layoutElement,
+ hasScreenPos,
+ height: 0,
+ entry,
+ previousPositions,
+ })
+ } else {
+ debugConsole.log(
+ 'Entry wrapper is missing indicator, box or callout, so ignoring',
+ wrapper
+ )
+ }
}
- const indicator = wrapper.querySelector(
- '.rp-entry-indicator'
+ if (entryViews.length === 0) {
+ resetLayout()
+ return
+ }
+
+ entryViews.sort((a, b) => a.entry.offset - b.entry.offset)
+
+ // Do the DOM interaction in three phases:
+ //
+ // - Apply the `display` property to all elements whose visibility has
+ // changed. This needs to happen first in order to measure heights.
+ // - Measure the height of each entry
+ // - Move each entry without animation to their original position
+ // relative to the editor content
+ // - Re-enable animation and position each entry
+ //
+ // The idea is to batch DOM reads and writes to avoid layout thrashing. In
+ // this case, the best we can do is a write phase, a read phase then a
+ // final write phase.
+ // See https://web.dev/avoid-large-complex-layouts-and-layout-thrashing/
+
+ // First, update display for each entry that needs it
+ hideOrShowEntries(entryViews)
+
+ // Next, measure the height of each entry
+ for (const entryView of entryViews) {
+ if (entryView.hasScreenPos) {
+ entryView.height = entryView.layout.offsetHeight
+ }
+ }
+
+ // Calculate positions for all positioned entries, starting by calculating
+ // which entry to put in its desired position and anchor everything else
+ // around. If there is an explicitly focused entry, use that.
+ let focusedEntryIndex = entryViews.findIndex(view => view.entry.focused)
+ if (focusedEntryIndex === -1) {
+ // There is no explicitly focused entry, so use the focused entry from the
+ // previous layout. This will be the first entry in the list if there was
+ // no previous layout.
+ focusedEntryIndex = Math.min(
+ previousLayoutInfoRef.current.focusedEntryIndex,
+ entryViews.length - 1
+ )
+ // If the entry from the previous layout has no screen position, fall back
+ // to the first entry in the list that does.
+ if (!entryViews[focusedEntryIndex].hasScreenPos) {
+ focusedEntryIndex = entryViews.findIndex(view => view.hasScreenPos)
+ }
+ }
+
+ // If there is no entry with a screen position, bail out
+ if (focusedEntryIndex === -1) {
+ return
+ }
+
+ const focusedEntryView = entryViews[focusedEntryIndex]
+
+ // If the focused entry has no screenPos, we can't position other
+ // entryViews relative to it, so we position all other entryViews as
+ // though the focused entry is at the top and the rest follow it
+ const entryViewsAfter = focusedEntryView.hasScreenPos
+ ? entryViews.slice(focusedEntryIndex + 1)
+ : [...entryViews]
+ const entryViewsBefore = focusedEntryView.hasScreenPos
+ ? entryViews.slice(0, focusedEntryIndex).reverse() // Work through backwards, starting with the one just above
+ : []
+
+ debugConsole.log('focusedEntryIndex', focusedEntryIndex)
+
+ let lastEntryBottom = 0
+ let firstEntryTop = 0
+
+ // Put the focused entry as close as possible to where it wants to be
+ if (focusedEntryView.hasScreenPos) {
+ const focusedEntryScreenPos = focusedEntryView.entry.screenPos
+ const entryTop = Math.max(focusedEntryScreenPos.y, toolbarPaddedHeight)
+ updateEntryPositions(focusedEntryView, entryTop, lineHeight)
+ lastEntryBottom = entryTop + focusedEntryView.height
+ firstEntryTop = entryTop
+ }
+
+ // Calculate positions for entries that are below the focused entry
+ calculateEntryViewPositions(
+ entryViewsAfter,
+ lineHeight,
+ (originalTop: number, height: number) => {
+ const top = Math.max(originalTop, lastEntryBottom + padding)
+ lastEntryBottom = top + height
+ return top
+ }
)
- const box = wrapper.querySelector('.rp-entry')
- const callout = wrapper.querySelector('.rp-entry-callout')
- const layoutElement = reviewPanelOpen ? box : indicator
- if (box && callout && layoutElement) {
- const previousPositions = previousLayoutInfoRef.current?.positions.find(
- pos => pos.entryId === entryId
- )?.positions
- const hasScreenPos = Boolean(entry.screenPos)
- entryViews.push({
- entryId,
- wrapper,
- indicator,
- box,
- callout,
- layout: layoutElement,
- hasScreenPos,
- height: 0,
- entry,
- previousPositions,
+ // Calculate positions for entries that are above the focused entry
+ calculateEntryViewPositions(
+ entryViewsBefore,
+ lineHeight,
+ (originalTop: number, height: number) => {
+ const originalBottom = originalTop + height
+ const bottom = Math.min(originalBottom, firstEntryTop - padding)
+ const top = bottom - height
+ firstEntryTop = top
+ return top
+ }
+ )
+
+ // Calculate the new top overflow
+ const overflowTop = Math.max(0, toolbarPaddedHeight - firstEntryTop)
+
+ // Check whether the positions of any entry have changed since the last
+ // layout
+ const positions = entryViews.map(
+ (entryView): EntryPositions => ({
+ entryId: entryView.entryId,
+ positions: entryView.positions,
})
- } else {
- debugConsole.log(
- 'Entry wrapper is missing indicator, box or callout, so ignoring',
- wrapper
+ )
+
+ const positionsChanged = !positionsEqual(
+ previousLayoutInfoRef.current.positions,
+ positions
+ )
+
+ // Check whether the top overflow or review panel height have changed
+ const overflowTopChanged =
+ overflowTop !== previousLayoutInfoRef.current.overflowTop
+
+ const height = lastEntryBottom + navPaddedHeight
+ const heightChanged = height !== previousLayoutInfoRef.current.height
+ const isMoveRequired = positionsChanged || overflowTopChanged
+
+ // Move entries into their initial positions, if animating, avoiding
+ // animation until the final animated move
+ if (animate && isMoveRequired) {
+ container.classList.add('no-animate')
+ moveEntriesToInitialPosition(entryViews, overflowTop)
+ }
+
+ // Inform the editor of the new top overflow and/or height if either has
+ // changed
+ if (overflowTopChanged || heightChanged) {
+ window.dispatchEvent(
+ new CustomEvent('review-panel:event', {
+ detail: {
+ type: 'sizes',
+ payload: {
+ overflowTop,
+ height,
+ },
+ },
+ })
)
}
- }
- if (entryViews.length === 0) {
- resetLayout()
- return
- }
+ // Do the final move
+ if (isMoveRequired) {
+ if (animate) {
+ container.classList.remove('no-animate')
+ moveEntriesToFinalPositions(entryViews, overflowTop, false)
+ } else {
+ container.classList.add('no-animate')
+ moveEntriesToFinalPositions(entryViews, overflowTop, true)
- entryViews.sort((a, b) => a.entry.offset - b.entry.offset)
+ // Force reflow now to ensure that entries are moved without animation
+ // eslint-disable-next-line no-void
+ void container.offsetHeight
- // Do the DOM interaction in three phases:
- //
- // - Apply the `display` property to all elements whose visibility has
- // changed. This needs to happen first in order to measure heights.
- // - Measure the height of each entry
- // - Move each entry without animation to their original position
- // relative to the editor content
- // - Re-enable animation and position each entry
- //
- // The idea is to batch DOM reads and writes to avoid layout thrashing. In
- // this case, the best we can do is a write phase, a read phase then a
- // final write phase.
- // See https://web.dev/avoid-large-complex-layouts-and-layout-thrashing/
+ container.classList.remove('no-animate')
+ }
+ }
- // First, update display for each entry that needs it
- hideOrShowEntries(entryViews)
+ previousLayoutInfoRef.current = {
+ positions,
+ focusedEntryIndex,
+ height,
+ overflowTop,
+ }
+ },
+ [entries, lineHeight, navHeight, reviewPanelOpen, toolbarHeight]
+ )
- // Next, measure the height of each entry
- for (const entryView of entryViews) {
- if (entryView.hasScreenPos) {
- entryView.height = entryView.layout.offsetHeight
+ useLayoutEffect(() => {
+ const callback = (event: Event) => {
+ const e = event as CustomEvent
+
+ if (!layoutSuspended) {
+ // Clear previous positions if forcing a layout
+ if (e.detail.force) {
+ previousLayoutInfoRef.current = initialLayoutInfo
+ }
+ layout(e.detail.animate)
}
}
- // Calculate positions for all positioned entries, starting by calculating
- // which entry to put in its desired position and anchor everything else
- // around. If there is an explicitly focused entry, use that.
- let focusedEntryIndex = entryViews.findIndex(view => view.entry.focused)
- if (focusedEntryIndex === -1) {
- // There is no explicitly focused entry, so use the focused entry from the
- // previous layout. This will be the first entry in the list if there was
- // no previous layout.
- focusedEntryIndex = Math.min(
- previousLayoutInfoRef.current.focusedEntryIndex,
- entryViews.length - 1
- )
- // If the entry from the previous layout has no screen position, fall back
- // to the first entry in the list that does.
- if (!entryViews[focusedEntryIndex].hasScreenPos) {
- focusedEntryIndex = entryViews.findIndex(view => view.hasScreenPos)
- }
+ window.addEventListener('review-panel:layout', callback)
+
+ return () => {
+ window.removeEventListener('review-panel:layout', callback)
}
-
- // If there is no entry with a screen position, bail out
- if (focusedEntryIndex === -1) {
- return
- }
-
- const focusedEntryView = entryViews[focusedEntryIndex]
-
- // If the focused entry has no screenPos, we can't position other
- // entryViews relative to it, so we position all other entryViews as
- // though the focused entry is at the top and the rest follow it
- const entryViewsAfter = focusedEntryView.hasScreenPos
- ? entryViews.slice(focusedEntryIndex + 1)
- : [...entryViews]
- const entryViewsBefore = focusedEntryView.hasScreenPos
- ? entryViews.slice(0, focusedEntryIndex).reverse() // Work through backwards, starting with the one just above
- : []
-
- debugConsole.log('focusedEntryIndex', focusedEntryIndex)
-
- let lastEntryBottom = 0
- let firstEntryTop = 0
-
- // Put the focused entry as close as possible to where it wants to be
- if (focusedEntryView.hasScreenPos) {
- const focusedEntryScreenPos = focusedEntryView.entry.screenPos
- const entryTop = Math.max(focusedEntryScreenPos.y, toolbarPaddedHeight)
- updateEntryPositions(focusedEntryView, entryTop, lineHeight)
- lastEntryBottom = entryTop + focusedEntryView.height
- firstEntryTop = entryTop
- }
-
- // Calculate positions for entries that are below the focused entry
- calculateEntryViewPositions(
- entryViewsAfter,
- lineHeight,
- (originalTop: number, height: number) => {
- const top = Math.max(originalTop, lastEntryBottom + padding)
- lastEntryBottom = top + height
- return top
- }
- )
-
- // Calculate positions for entries that are above the focused entry
- calculateEntryViewPositions(
- entryViewsBefore,
- lineHeight,
- (originalTop: number, height: number) => {
- const originalBottom = originalTop + height
- const bottom = Math.min(originalBottom, firstEntryTop - padding)
- const top = bottom - height
- firstEntryTop = top
- return top
- }
- )
-
- // Calculate the new top overflow
- const overflowTop = Math.max(0, toolbarPaddedHeight - firstEntryTop)
-
- // Check whether the positions of any entry have changed since the last
- // layout
- const positions = entryViews.map(
- (entryView): EntryPositions => ({
- entryId: entryView.entryId,
- positions: entryView.positions,
- })
- )
-
- const positionsChanged = !positionsEqual(
- previousLayoutInfoRef.current.positions,
- positions
- )
-
- // Check whether the top overflow or review panel height have changed
- const overflowTopChanged =
- overflowTop !== previousLayoutInfoRef.current.overflowTop
-
- const height = lastEntryBottom + navPaddedHeight
- const heightChanged = height !== previousLayoutInfoRef.current.height
- const isMoveRequired = positionsChanged || overflowTopChanged
-
- // Move entries into their initial positions, if animating, avoiding
- // animation until the final animated move
- if (animate && isMoveRequired) {
- container.classList.add('no-animate')
- moveEntriesToInitialPosition(entryViews, overflowTop)
- }
-
- // Inform the editor of the new top overflow and/or height if either has
- // changed
- if (overflowTopChanged || heightChanged) {
- window.dispatchEvent(
- new CustomEvent('review-panel:event', {
- detail: {
- type: 'sizes',
- payload: {
- overflowTop,
- height,
- },
- },
- })
- )
- }
-
- // Do the final move
- if (isMoveRequired) {
- if (animate) {
- container.classList.remove('no-animate')
- moveEntriesToFinalPositions(entryViews, overflowTop, false)
- } else {
- container.classList.add('no-animate')
- moveEntriesToFinalPositions(entryViews, overflowTop, true)
-
- // Force reflow now to ensure that entries are moved without animation
- // eslint-disable-next-line no-void
- void container.offsetHeight
-
- container.classList.remove('no-animate')
- }
- }
-
- previousLayoutInfoRef.current = {
- positions,
- focusedEntryIndex,
- height,
- overflowTop,
- }
- }
-
- useEventListener('review-panel:layout', (event: CustomEvent) => {
- if (!layoutSuspended) {
- // Clear previous positions if forcing a layout
- if (event.detail.force) {
- previousLayoutInfoRef.current = initialLayoutInfo
- }
- layout(event.detail.animate)
- }
- })
+ }, [layoutSuspended, layout])
// Layout on first render. This is necessary to ensure layout happens when
// switching from overview to current file view
diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx
index 1ab93329f1..4b0ad7d976 100644
--- a/services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx
+++ b/services/web/frontend/js/features/source-editor/components/review-panel/toggler.tsx
@@ -1,5 +1,8 @@
import { useTranslation } from 'react-i18next'
import { useReviewPanelUpdaterFnsContext } from '../../context/review-panel/review-panel-context'
+import Icon from '@/shared/components/icon'
+import MaterialIcon from '@/shared/components/material-icon'
+import BootstrapVersionSwitcher from '@/features/ui/components/bootstrap-5/bootstrap-version-switcher'
function Toggler() {
const { t } = useTranslation()
@@ -18,6 +21,12 @@ function Toggler() {
onClick={handleTogglerClick}
>
{t('hotkey_toggle_review_panel')}
+
+ }
+ bs5={}
+ />
+
)
}
diff --git a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx b/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx
index 6e03282cf8..81550e5b37 100644
--- a/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx
+++ b/services/web/frontend/js/features/source-editor/components/review-panel/toolbar/resolved-comments-dropdown.tsx
@@ -1,7 +1,6 @@
import { useTranslation } from 'react-i18next'
import { useState, useMemo, useCallback } from 'react'
import Icon from '../../../../../shared/components/icon'
-import Tooltip from '../../../../../shared/components/tooltip'
import ResolvedCommentsScroller from './resolved-comments-scroller'
import classnames from 'classnames'
import {
@@ -16,6 +15,10 @@ import { ReviewPanelResolvedCommentThread } from '../../../../../../../types/rev
import { DocId } from '../../../../../../../types/project-settings'
import { ReviewPanelEntry } from '../../../../../../../types/review-panel/entry'
import { useFileTreeData } from '@/shared/context/file-tree-data-context'
+import LoadingSpinner from '@/shared/components/loading-spinner'
+import OLTooltip from '@/features/ui/components/ol/ol-tooltip'
+import MaterialIcon from '@/shared/components/material-icon'
+import BootstrapVersionSwitcher from '@/features/ui/components/bootstrap-5/bootstrap-version-switcher'
export interface FilteredResolvedComments
extends ReviewPanelResolvedCommentThread {
@@ -97,7 +100,7 @@ function ResolvedCommentsDropdown() {
onClick={() => setIsOpen(false)}
/>
-
+
{isLoading ? (
-
-
-
+
) : isOpen ? (
))}
{!resolvedComments.length && (
-
{t('no_resolved_threads')}
+
{t('no_resolved_threads')}
)}
-
-
{t('tc_everyone')}
-
+
{Object.values(formattedProjectMembers).map(member => (
-
-
{member.name}
-
+
-
-
{t('tc_guests')}
-
+
setShow(false)}>
-
-
{t('upgrade_to_track_changes')}
-
-
+ setShow(false)}>
+
+ {t('upgrade_to_track_changes')}
+
+
{/* eslint-disable-next-line jsx-a11y/media-has-caption */}