[cm6] Improve alignment of review panel elements (#12710)

* Use a block widget for top padding
* Update review panel positions when the editor geometry changes
* Remove editorPaddingTop from position calculations
* Recalculate review panel on start adding comment
* Assert on line content rather than index
* Use broadcastChange
* Keep focus in the editor when opening the review panel
* debounce broadcastChange
* Set CULL_AFTER to Infinity

GitOrigin-RevId: a8d7b8967736a9164b5264eeaadf334c15ec95ce
This commit is contained in:
Alf Eaton 2023-04-24 10:33:43 +01:00 committed by Copybot
parent acf6abb0fb
commit 08ccdb79d3
6 changed files with 21 additions and 19 deletions

View file

@ -28,7 +28,7 @@
a.rp-add-comment-btn( a.rp-add-comment-btn(
href href
ng-if="reviewPanel.entries[editor.open_doc_id]['add-comment'] != null && permissions.comment" ng-if="reviewPanel.entries[editor.open_doc_id]['add-comment'] != null && permissions.comment"
ng-click="addNewComment();" ng-mousedown="addNewComment($event);"
) )
i.fa.fa-comment i.fa.fa-comment
|  #{translate("add_comment")} |  #{translate("add_comment")}

View file

@ -78,7 +78,10 @@ const EditorNavigationToolbarRoot = React.memo(
}, [chatIsOpen, setChatIsOpen, markMessagesAsRead]) }, [chatIsOpen, setChatIsOpen, markMessagesAsRead])
const toggleReviewPanelOpen = useCallback( const toggleReviewPanelOpen = useCallback(
() => setReviewPanelOpen(value => !value), event => {
event.preventDefault()
setReviewPanelOpen(value => !value)
},
[setReviewPanelOpen] [setReviewPanelOpen]
) )

View file

@ -11,7 +11,7 @@ function TrackChangesToggleButton({ trackChangesIsOpen, disabled, onClick }) {
return ( return (
<div className="toolbar-item"> <div className="toolbar-item">
<button disabled={disabled} className={classes} onClick={onClick}> <button disabled={disabled} className={classes} onMouseDown={onClick}>
<i className="review-icon" /> <i className="review-icon" />
<p className="toolbar-label">{t('review')}</p> <p className="toolbar-label">{t('review')}</p>
</button> </button>

View file

@ -13,7 +13,7 @@ import { debounce } from 'lodash'
// With less than this number of entries, don't bother culling to avoid // With less than this number of entries, don't bother culling to avoid
// little UI jumps when scrolling. // little UI jumps when scrolling.
const CULL_AFTER = 100 const CULL_AFTER = Infinity // Note: was 100 but couldn't scroll to see items outside the viewport
export const dispatchEditorEvent = (type: string, payload?: unknown) => { export const dispatchEditorEvent = (type: string, payload?: unknown) => {
window.setTimeout(() => { window.setTimeout(() => {
@ -291,24 +291,25 @@ export const createChangeManager = (
acceptChanges(payload) acceptChanges(payload)
view.dispatch(buildChangeMarkers()) view.dispatch(buildChangeMarkers())
broadcastChange() broadcastChange()
dispatchFocusChangedEvent(view.state)
break break
} }
case 'changes:reject': { case 'changes:reject': {
view.dispatch(rejectChanges(payload)) view.dispatch(rejectChanges(payload))
dispatchFocusChangedEvent(view.state) broadcastChange()
break break
} }
case 'comment:select_line': { case 'comment:select_line': {
selectCurrentLine() selectCurrentLine()
broadcastChange()
break break
} }
case 'comment:add': { case 'comment:add': {
addComment(payload.offset, payload.length, payload.threadId) addComment(payload.offset, payload.length, payload.threadId)
collapseSelection() collapseSelection()
broadcastChange()
break break
} }
@ -329,9 +330,6 @@ export const createChangeManager = (
case 'loaded_threads': { case 'loaded_threads': {
view.dispatch(buildChangeMarkers()) view.dispatch(buildChangeMarkers())
broadcastChange() broadcastChange()
window.setTimeout(() => {
dispatchFocusChangedEvent(view.state)
}, 0)
break break
} }
@ -359,12 +357,9 @@ export const createChangeManager = (
} }
} }
/** const broadcastChange = debounce(() => {
*
*/
const broadcastChange = () => {
dispatchEditorEvent('track-changes:changed') dispatchEditorEvent('track-changes:changed')
} }, 50)
/** /**
* When the editor content, focus, size, viewport or selection changes, * When the editor content, focus, size, viewport or selection changes,
@ -450,9 +445,13 @@ export const createChangeManager = (
return { return {
initialize() { initialize() {
addListeners() addListeners()
dispatchEditorEvent('track-changes:changed') broadcastChange()
}, },
handleUpdate(update: ViewUpdate) { handleUpdate(update: ViewUpdate) {
if (update.geometryChanged && !update.docChanged) {
broadcastChange()
}
if (updateSetsVerticalOverflow(update)) { if (updateSetsVerticalOverflow(update)) {
ignoreGeometryChangesUntil = Date.now() + 50 // ignore changes for 50ms ignoreGeometryChangesUntil = Date.now() + 50 // ignore changes for 50ms
} else if ( } else if (

View file

@ -724,7 +724,8 @@ export default App.controller(
return $scope.toggleReviewPanel() return $scope.toggleReviewPanel()
} }
$scope.addNewComment = function () { $scope.addNewComment = function (e) {
e.preventDefault()
$scope.$broadcast('comment:start_adding') $scope.$broadcast('comment:start_adding')
return $scope.toggleReviewPanel() return $scope.toggleReviewPanel()
} }

View file

@ -146,12 +146,11 @@ export default App.directive('reviewPanelSorted', $timeout => ({
visibility: 'visible', visibility: 'visible',
}) })
focused_entry.$indicator_el.css({ top: focused_entry_top }) focused_entry.$indicator_el.css({ top: focused_entry_top })
// use screenPos.height and screenPos.editorPaddingTop if set // use screenPos.height if set
screenPosHeight = focusedEntryScreenPos.height ?? line_height screenPosHeight = focusedEntryScreenPos.height ?? line_height
positionLayoutEl( positionLayoutEl(
focused_entry.$callout_el, focused_entry.$callout_el,
focusedEntryScreenPos.y + focusedEntryScreenPos.y,
(focusedEntryScreenPos.editorPaddingTop ?? 0),
focused_entry_top, focused_entry_top,
screenPosHeight screenPosHeight
) )