LEAN-5459 Pick and Highlight changes of a single author by mbartenev-atypon · Pull Request #1142 · Atypon-OpenSource/manuscripts-article-editor

mbartenev-atypon · 2026-04-10T12:25:18Z

Added authors picker.
Introduced compact mode for the header
Refactored header a bit to fit in new components

…icle-editor into LEAN-5459

Copilot

Pull request overview

Adds UI support for selecting a single collaborator/author and highlighting their tracked changes, alongside a refactor of the manuscript editor header to support a compact layout.

Changes:

Introduces a new EditorHeaderGroup component (with compact mode) and wires it into ManuscriptPageContainer.
Adds collaborator roster + dropdown components to toggle an “author highlight” state in the editor.
Updates dependency lockfile content (currently includes local link: overrides).

Reviewed changes

Copilot reviewed 7 out of 8 changed files in this pull request and generated 12 comments.

Show a summary per file

File	Description
src/components/track-changes/utils.ts	Adds TODO/commentary near suggestion selection logic.
src/components/track-changes/SortByDropdown.tsx	Updates header copyright year.
src/components/projects/ManuscriptPageContainer.tsx	Refactors header rendering to use `EditorHeaderGroup`.
src/components/projects/EditorHeaderGroup.tsx	New combined header component with compact mode + track-changes toggle + collaborators roster.
src/components/projects/CollaboratorsRoster.tsx	New roster UI that toggles highlighted author via editor plugin meta.
src/components/projects/CollaboratorsDropdownRoster.tsx	New dropdown UI for overflow collaborators.
pnpm-lock.yaml	Adds `overrides` + `link:`-based dependency resolution (machine-local paths).
package.json	Minor ordering/formatting change in devDependencies.

Files not reviewed (1)

pnpm-lock.yaml: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-14T11:38:48Z

+    const users = roster.length ? [...roster] : [...usersDict.values()]
+    const highlightedAuthor = highlightedAuthorId
+      ? usersDict.get(highlightedAuthorId)
+      : undefined
+    let pos = maxInMainRoster - 1
+
+    if (highlightedAuthor && users.indexOf(highlightedAuthor) > pos) {
+      // offsetting items when the selected user is in the dropdown has to move into the main roster to provide nicer UX
+      // insted of just swapping with the last in the main roster and making the rest look stale
+      while (users.indexOf(highlightedAuthor) != pos) {
+        const current = users.indexOf(highlightedAuthor)
+        const swap = current == users.length - 1 ? 0 : current + 1
+        ;[users[current], users[swap]] = [users[swap], users[current]]
+      }
+    }
+    setRoster(users)


This effect uses the existing roster when roster.length is non-zero, which prevents newly discovered authors (from usersDict) from ever being added after the first run. As a result the roster can become stale when new pending changes appear. Consider always deriving users from usersDict.values() and then applying your reordering logic on top of that (preserving previous order where possible).

Suggested change

const users = roster.length ? [...roster] : [...usersDict.values()]

const highlightedAuthor = highlightedAuthorId

? usersDict.get(highlightedAuthorId)

: undefined

let pos = maxInMainRoster - 1

if (highlightedAuthor && users.indexOf(highlightedAuthor) > pos) {

// offsetting items when the selected user is in the dropdown has to move into the main roster to provide nicer UX

// insted of just swapping with the last in the main roster and making the rest look stale

while (users.indexOf(highlightedAuthor) != pos) {

const current = users.indexOf(highlightedAuthor)

const swap = current == users.length - 1 ? 0 : current + 1

;[users[current], users[swap]] = [users[swap], users[current]]

}

}

setRoster(users)

setRoster((prevRoster) => {

const latestUsers = [...usersDict.values()]

const users = [

...prevRoster.filter((user) => latestUsers.includes(user)),

...latestUsers.filter((user) => !prevRoster.includes(user)),

]

const highlightedAuthor = highlightedAuthorId

? usersDict.get(highlightedAuthorId)

: undefined

const pos = maxInMainRoster - 1

if (highlightedAuthor && users.indexOf(highlightedAuthor) > pos) {

// offsetting items when the selected user is in the dropdown has to move into the main roster to provide nicer UX

// insted of just swapping with the last in the main roster and making the rest look stale

while (users.indexOf(highlightedAuthor) != pos) {

const current = users.indexOf(highlightedAuthor)

const swap = current == users.length - 1 ? 0 : current + 1

;[users[current], users[swap]] = [users[swap], users[current]]

}

}

return users

})

Copilot · 2026-04-14T11:38:48Z

+      return {
+        trackState: state.trackState,
+        collaboratorsById: state.collaboratorsById,
+        permittedActions: state.permittedActions,
+        isViewingMode: state.isViewingMode,
+        editor: state.editor,
+        isActive: state.isTrackingChangesVisible,
+        project: state.project,
+      }


The useStore selector is subscribing to permittedActions and isViewingMode, but those values are never used by this component. This creates unnecessary re-renders when those fields change. Please remove unused fields from the selector return value.

Copilot · 2026-04-14T11:38:49Z

+import React, { useCallback, useMemo, useState, useEffect } from 'react'
+import styled from 'styled-components'
+
+import { useStore } from '../../store/useStore'


This file imports useStore directly from ../../store/useStore, but the project’s barrel export already re-exports it (src/store/index.ts exports ./useStore). Prefer importing from ../../store to keep imports consistent and make refactors easier.

Suggested change

import { useStore } from '../../store/useStore'

import { useStore } from '../../store'

Copilot · 2026-04-14T11:38:49Z

+export function GetName(user: UserProfile, project: Project, full = false) {
+  // @ts-ignore
+  const name = user.bibliographicName?.given as string
+
+  if (!name) {
+    return getUserRole(project, user.userID)
+  }
+  return full ? name : name[0]
+}
+export function GetSurname(
+  user: UserProfile,


GetName/GetSurname are regular helper functions but are named like React components (PascalCase). This is inconsistent with typical function naming and makes callsites harder to scan. Consider renaming them to getName/getSurname (and updating imports/usages).

Copilot · 2026-04-14T11:38:49Z

+      <DropdownButtonContainer
+        ref={toggleButtonRef}
+        isOpen={isOpen}
+        onClick={toggleOpen}
+        onKeyDown={(e) => {
+          if (e.key === 'ArrowDown') {
+            toggleOpen()
+          }
+        }}
+        className={'dropdown-toggle'}
+        tabIndex={0}
+      >
+        +{roster.length}
+      </DropdownButtonContainer>
+      {isOpen && (
+        <Dropdown minWidth={100}>
+          <RosterDropdown>
+            {roster.map(
+              (u, i) =>
+                u &&
+                u._id && (
+                  <DropdownItem
+                    type="button"
+                    key={u._id}
+                    onClick={() => onUserClick(u._id)}
+                    aria-label={`Show changes made by ${GetName(u, project, true) + ' ' + GetSurname(u, collaboratorsById, true)}`}
+                  >
+                    <UserIconDisplay>
+                      {GetName(u, project)}
+                      {GetSurname(u, collaboratorsById)}
+                    </UserIconDisplay>
+                    <span>
+                      {GetName(u, project, true)}{' '}
+                      {GetSurname(u, collaboratorsById, true)}
+                    </span>
+                  </DropdownItem>
+                )
+            )}
+          </RosterDropdown>
+        </Dropdown>


When restRoster is empty this still renders a toggle button showing +0, which is confusing and creates an unnecessary focus target. Consider rendering this dropdown button only when roster.length > 0.

Suggested change

<DropdownButtonContainer

ref={toggleButtonRef}

isOpen={isOpen}

onClick={toggleOpen}

onKeyDown={(e) => {

if (e.key === 'ArrowDown') {

toggleOpen()

}

}}

className={'dropdown-toggle'}

tabIndex={0}

>

+{roster.length}

</DropdownButtonContainer>

{isOpen && (

<Dropdown minWidth={100}>

<RosterDropdown>

{roster.map(

(u, i) =>

u &&

u._id && (

<DropdownItem

type="button"

key={u._id}

onClick={() => onUserClick(u._id)}

aria-label={`Show changes made by ${GetName(u, project, true) + ' ' + GetSurname(u, collaboratorsById, true)}`}

>

<UserIconDisplay>

{GetName(u, project)}

{GetSurname(u, collaboratorsById)}

</UserIconDisplay>

<span>

{GetName(u, project, true)}{' '}

{GetSurname(u, collaboratorsById, true)}

</span>

</DropdownItem>

)

)}

</RosterDropdown>

</Dropdown>

{roster.length > 0 && (

<>

<DropdownButtonContainer

ref={toggleButtonRef}

isOpen={isOpen}

onClick={toggleOpen}

onKeyDown={(e) => {

if (e.key === 'ArrowDown') {

toggleOpen()

}

}}

className={'dropdown-toggle'}

tabIndex={0}

>

+{roster.length}

</DropdownButtonContainer>

{isOpen && (

<Dropdown minWidth={100}>

<RosterDropdown>

{roster.map(

(u, i) =>

u &&

u._id && (

<DropdownItem

type="button"

key={u._id}

onClick={() => onUserClick(u._id)}

aria-label={`Show changes made by ${GetName(u, project, true) + ' ' + GetSurname(u, collaboratorsById, true)}`}

>

<UserIconDisplay>

{GetName(u, project)}

{GetSurname(u, collaboratorsById)}

</UserIconDisplay>

<span>

{GetName(u, project, true)}{' '}

{GetSurname(u, collaboratorsById, true)}

</span>

</DropdownItem>

)

)}

</RosterDropdown>

</Dropdown>

)}

</>

Copilot · 2026-04-14T11:38:50Z

+
+    if (highlightedAuthor && users.indexOf(highlightedAuthor) > pos) {
+      // offsetting items when the selected user is in the dropdown has to move into the main roster to provide nicer UX
+      // insted of just swapping with the last in the main roster and making the rest look stale


Typo in comment: "insted" → "instead".

Suggested change

// insted of just swapping with the last in the main roster and making the rest look stale

// instead of just swapping with the last in the main roster and making the rest look stale

Copilot · 2026-04-14T11:38:50Z

+            aria-label="show tracked changes"
+            aria-pressed={isTrackingChangesVisible}
+            data-tooltip-content={compact ? label : ''}
+          >
+            {!compact && <Label>Show tracked changes</Label>}


The visible label and accessibility label are inconsistent with the current state: the text always renders "Show tracked changes" and aria-label is hardcoded, while the action toggles between show/hide. Use the computed label for the visible text (when not compact) and for aria-label so both reflect the current state.

Suggested change

aria-label="show tracked changes"

aria-pressed={isTrackingChangesVisible}

data-tooltip-content={compact ? label : ''}

>

{!compact && <Label>Show tracked changes</Label>}

aria-label={label}

aria-pressed={isTrackingChangesVisible}

data-tooltip-content={compact ? label : ''}

>

{!compact && <Label>{label}</Label>}

Copilot · 2026-04-14T11:38:51Z

  const view = editor.view
  const tr = state.tr
+  /* @TODO consider moving this logic into tc plugin */
+  /** do we actually need a specia case for structure changes? will it not be covered by the node selection? */


Typo in comment: "specia" → "special".

Suggested change

/** do we actually need a specia case for structure changes? will it not be covered by the node selection? */

/** do we actually need a special case for structure changes? will it not be covered by the node selection? */

Copilot · 2026-04-14T11:38:51Z

+export const ManuscriptMenusContainer = styled.div`
+  display: flex;
+  align-items: center;
+  justify-content: space-between;
+  background-color: ${(props) => props.theme.colors.background.secondary};
+`
+export const ManuscriptMenusContainerInner = styled.div`
+  width: 100%;
+  max-width: ${(props) => props.theme.grid.editorMaxWidth}px;
+  margin: 0 auto;
+  padding: ${(props) => props.theme.grid.unit / 2}px
+    ${(props) => props.theme.grid.unit * 5}px
+    ${(props) => props.theme.grid.unit}px
+    ${(props) => props.theme.grid.unit * 15}px;
+  box-sizing: border-box;
+`


ManuscriptMenusContainer / ManuscriptMenusContainerInner are now defined (and exported) here, but the same exports still exist in ManuscriptPageContainer.tsx. This duplication makes it unclear which should be imported going forward. Consider keeping a single source of truth (e.g., export them only from EditorHeaderGroup or move them to a shared styles module and re-export from both if backward compatibility is required).

Copilot · 2026-04-14T11:38:51Z

+overrides:
+  '@manuscripts/body-editor': link:../../../Library/pnpm/global/5/node_modules/@manuscripts/body-editor
+  '@manuscripts/track-changes-plugin': link:../../../Library/pnpm/global/5/node_modules/@manuscripts/track-changes-plugin
+


The lockfile introduces overrides and dependency specifiers that point to a developer-machine specific link:../../../Library/pnpm/global/... path. Committing these links will break installs/CI for other environments and also removes the resolved package snapshots from the lockfile. Please revert these links and pin @manuscripts/body-editor / @manuscripts/track-changes-plugin to published versions (or a workspace-local path within the repo if this is meant to be a monorepo link).

zax-assistant · 2026-04-15T09:11:18Z

Published version 4.13.19-LEAN-5459.0

mbartenev-atypon added 7 commits March 20, 2026 18:05

wip

dd73d1a

minor update

c242913

wip

879769a

Merge branch 'master' of github.com:Atypon-OpenSource/manuscripts-art…

46d5034

…icle-editor into LEAN-5459

wip

2d7af30

main ui added

95729e1

mostly done

24a64fb

mbartenev-atypon requested review from IslamJMomani and Copilot and removed request for Copilot April 10, 2026 12:25

Copilot started reviewing on behalf of mbartenev-atypon April 10, 2026 12:25 View session

IslamJMomani approved these changes Apr 14, 2026

View reviewed changes

Comment thread package.json Outdated

LEAN-5459 clean up on review

e08387e

Copilot AI review requested due to automatic review settings April 14, 2026 11:32

Copilot started reviewing on behalf of mbartenev-atypon April 14, 2026 11:33 View session

Copilot AI reviewed Apr 14, 2026

View reviewed changes

zax-assistant Bot added 3 commits April 15, 2026 09:05

Sync with master

8994345

Update package.json

160e469

Install dependencies

830e2f3

minor fixes

a6f1d23

Navigation Menu

LEAN-5459 Pick and Highlight changes of a single author - #1142

LEAN-5459 Pick and Highlight changes of a single author#1142
mbartenev-atypon wants to merge 12 commits into
masterfrom
LEAN-5459

mbartenev-atypon commented Apr 10, 2026 •

edited

Loading

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

Copilot AI Apr 14, 2026

Uh oh!

zax-assistant Bot commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	import { useStore } from '../../store/useStore'
	import { useStore } from '../../store'

	// insted of just swapping with the last in the main roster and making the rest look stale
	// instead of just swapping with the last in the main roster and making the rest look stale

	/** do we actually need a specia case for structure changes? will it not be covered by the node selection? */
	/** do we actually need a special case for structure changes? will it not be covered by the node selection? */

Conversation

mbartenev-atypon commented Apr 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 14, 2026

Choose a reason for hiding this comment

Uh oh!

zax-assistant Bot commented Apr 15, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mbartenev-atypon commented Apr 10, 2026 •

edited

Loading