fix(google-docs): improve highlight styling and consistency across view/edit mode [INTEG-3861]#10949
Conversation
b86c4eb to
f32c8e1
Compare
There was a problem hiding this comment.
Pull request overview
Improves mapping highlight styling in the Google Docs review UI and aligns behavior/UX between view vs edit mode, including how image mappings and the mapping rail behave.
Changes:
- Adjust highlight colors and text-segment highlight styling (background + padding) and update image card highlight styling.
- Make image “edit mapping” actions optional so they can be disabled in view mode.
- Refactor view-mode rendering to use
ViewMappingRailalongside the document content.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| apps/google-docs/src/locations/Page/components/review/mapping/documentRenderers.tsx | Updates highlight styling, makes onEditImage optional, and simplifies table rendering/hover behavior. |
| apps/google-docs/src/locations/Page/components/review/mapping/ViewMappingRail.tsx | Minor variable rename inside hover detection. |
| apps/google-docs/src/locations/Page/components/review/mapping/ReviewImageAssetCard.tsx | Makes onEdit optional and updates highlight visuals/actions rendering. |
| apps/google-docs/src/locations/Page/components/review/mapping/NormalizedDocumentSection.tsx | Makes onEditImage optional (but currently leaves an isViewMode mismatch). |
| apps/google-docs/src/locations/Page/components/review/mapping/MappingView.tsx | Refactors view-mode layout to use ViewMappingRail and adds a small UI tip. |
| apps/google-docs/src/locations/Page/components/review/mapping/MappingCard.tsx | Changes card border styling for hover state (currently missing an explicit color). |
Comments suppressed due to low confidence (1)
apps/google-docs/src/locations/Page/components/review/mapping/MappingCard.tsx:48
borderis now set to a shorthand value without a color (e.g. "1px solid"), which will default tocurrentColorand likely render an unintended border (and the hover state no longer changes border color). If the intent is to keep the previous green border-on-hover behavior, reintroduce an explicit color; if the intent is no border, set it totransparent/noneand update the transition accordingly.
border: `${isHovered ? 2 : 1}px solid`,
borderRadius: tokens.borderRadiusMedium,
padding: tokens.spacing2Xs,
backgroundColor: tokens.green100,
transition: 'border-color 120ms ease, border-width 120ms ease',
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| import { buildMappingDisplayGroups } from './buildMappingDisplayGroups'; | ||
| import { type ViewMappingCardEntry } from './ViewMappingRail'; | ||
| import { ViewMappingRail, type ViewMappingCardEntry } from './ViewMappingRail'; | ||
| import { ViewMappingCard } from './ViewMappingCard'; |
There was a problem hiding this comment.
ViewMappingCard is imported but no longer used after switching view mode rendering to ViewMappingRail. This will fail linting/TS checks in many setups; remove the unused import (or use it directly if still needed).
| import { ViewMappingCard } from './ViewMappingCard'; |
| {row.cells.map((cell) => ( | ||
| <TableCell | ||
| key={cell.id} | ||
| data-testid={`table-cell-${cell.id}`} | ||
| style={{ backgroundColor: 'transparent', verticalAlign: 'top' }}> | ||
| <Flex flexDirection="column" gap="spacing2Xs"> | ||
| {cell.parts.map((part) => { | ||
| const partKey = [table.id, row.id, cell.id, part.id].join(':'); | ||
| return ( | ||
| <Box key={part.id}> | ||
| <TablePartRenderer | ||
| segmentId={segmentId} | ||
| tableId={table.id} | ||
| rowId={row.id} | ||
| cellId={cell.id} | ||
| part={part} | ||
| visibleHighlights={getVisiblePartHighlights(partKey)} | ||
| imageById={imageById} | ||
| excludedSourceRefs={excludedSourceRefs} | ||
| hoveredMappingKeys={hoveredMappingKeys} | ||
| onSetHoveredMappingKeys={onSetHoveredMappingKeys} | ||
| onEditImage={onEditImage} | ||
| /> | ||
| </Box> | ||
| ); | ||
| })} | ||
| </Flex> | ||
| </TableCell> | ||
| ))} |
There was a problem hiding this comment.
After removing the table-cell hover/border wrapper, TableRenderer no longer uses the per-cell mapping key computation. As a result, borderIndex/getCellMappingKeys (and the fullHighlightIndex prop/comment) appear to be dead code now. Please remove the unused helpers/props or reintroduce the missing usage; leaving them in place risks unused-variable failures and makes the renderer harder to maintain.
| {row.cells.map((cell) => ( | |
| <TableCell | |
| key={cell.id} | |
| data-testid={`table-cell-${cell.id}`} | |
| style={{ backgroundColor: 'transparent', verticalAlign: 'top' }}> | |
| <Flex flexDirection="column" gap="spacing2Xs"> | |
| {cell.parts.map((part) => { | |
| const partKey = [table.id, row.id, cell.id, part.id].join(':'); | |
| return ( | |
| <Box key={part.id}> | |
| <TablePartRenderer | |
| segmentId={segmentId} | |
| tableId={table.id} | |
| rowId={row.id} | |
| cellId={cell.id} | |
| part={part} | |
| visibleHighlights={getVisiblePartHighlights(partKey)} | |
| imageById={imageById} | |
| excludedSourceRefs={excludedSourceRefs} | |
| hoveredMappingKeys={hoveredMappingKeys} | |
| onSetHoveredMappingKeys={onSetHoveredMappingKeys} | |
| onEditImage={onEditImage} | |
| /> | |
| </Box> | |
| ); | |
| })} | |
| </Flex> | |
| </TableCell> | |
| ))} | |
| {row.cells.map((cell) => { | |
| const cellMappingKeys = getCellMappingKeys(table.id, row.id, cell); | |
| const isCellHovered = cellMappingKeys.some((mappingKey) => | |
| hoveredMappingKeys.includes(mappingKey), | |
| ); | |
| const hasCellBorderHighlight = cellMappingKeys.some((mappingKey) => Boolean(borderIndex[mappingKey])); | |
| const hasCellFullHighlight = cellMappingKeys.some((mappingKey) => | |
| Boolean(fullHighlightIndex[mappingKey]), | |
| ); | |
| return ( | |
| <TableCell | |
| key={cell.id} | |
| data-testid={`table-cell-${cell.id}`} | |
| style={{ backgroundColor: 'transparent', verticalAlign: 'top' }}> | |
| <Box | |
| onMouseEnter={() => onSetHoveredMappingKeys(cellMappingKeys)} | |
| onMouseLeave={() => onSetHoveredMappingKeys([])} | |
| style={{ | |
| border: | |
| isCellHovered || hasCellBorderHighlight || hasCellFullHighlight | |
| ? `1px solid ${tokens.colorBlue400}` | |
| : undefined, | |
| borderRadius: tokens.borderRadiusMedium, | |
| padding: tokens.spacing2Xs, | |
| }}> | |
| <Flex flexDirection="column" gap="spacing2Xs"> | |
| {cell.parts.map((part) => { | |
| const partKey = [table.id, row.id, cell.id, part.id].join(':'); | |
| return ( | |
| <Box key={part.id}> | |
| <TablePartRenderer | |
| segmentId={segmentId} | |
| tableId={table.id} | |
| rowId={row.id} | |
| cellId={cell.id} | |
| part={part} | |
| visibleHighlights={getVisiblePartHighlights(partKey)} | |
| imageById={imageById} | |
| excludedSourceRefs={excludedSourceRefs} | |
| hoveredMappingKeys={hoveredMappingKeys} | |
| onSetHoveredMappingKeys={onSetHoveredMappingKeys} | |
| onEditImage={onEditImage} | |
| /> | |
| </Box> | |
| ); | |
| })} | |
| </Flex> | |
| </Box> | |
| </TableCell> | |
| ); | |
| })} |
135a569 to
55493f4
Compare
55493f4 to
67cc2c6
Compare
Purpose
Improves highlight styling consistency and adds hover-based highlighting to view mode in
the Google Docs mapping review UI. Previously, view mode suppressed all highlighting; now
cards and document segments respond to hover the same way edit mode does.
Summary
EMPTY_HIGHLIGHT_INDEXoverride so view modehighlights on hover like edit mode
hoveredMappingKeysintoViewMappingRail→ViewMappingCard; card border thickens on hoveronEditImage: Made optional throughout — view mode image cards renderwithout an actions menu
transition (
green100→green300)green200→green100; border-radiusswapped for vertical padding
LightbulbIconScreen.Recording.2026-04-29.at.2.49.04.PM.mov
Testing Steps
document content highlights (background turns green, card border thickens).
actions menu.
the "Currently viewing" label.