Skip to content

Commit

Permalink
[FIX] popover: adjusted z-index for the popovers
Browse files Browse the repository at this point in the history
Previously, the Popovers component had a higher z-index value than the
Grid Composer component. As a result, Popovers would appear on top of the
Composer Assistant.

To resolve this issue, a new enum has been introduced called GridPopover.
Whenever a call is made from grid_popover.ts, the z-index value will be set to
GridPopover enum. Otherwise, the z-index value for Popovers will remain
the same i.e. 30.

Task-3211697

closes #2206

Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
dhrp-odoo committed Mar 28, 2023
1 parent 07582df commit 8c89731
Show file tree
Hide file tree
Showing 7 changed files with 20 additions and 9 deletions.
2 changes: 2 additions & 0 deletions src/components/grid_popover/grid_popover.ts
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Component } from "@odoo/owl";
import { ComponentsImportance } from "../../constants";
import { DOMCoordinates, Position, SpreadsheetChildEnv } from "../../types";
import { ClosedCellPopover, PositionedCellPopover } from "../../types/cell_popovers";
import { Popover } from "../popover/popover";
Expand All @@ -12,6 +13,7 @@ interface Props {
export class GridPopover extends Component<Props, SpreadsheetChildEnv> {
static template = "o-spreadsheet-GridPopover";
static components = { Popover };
zIndex = ComponentsImportance.GridPopover;

get cellPopover(): PositionedCellPopover | ClosedCellPopover {
const popover = this.env.model.getters.getCellPopover(this.props.hoveredCell);
Expand Down
3 changes: 2 additions & 1 deletion src/components/grid_popover/grid_popover.xml
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,8 @@
childHeight="cellPopover.Component.size.height"
flipHorizontalOffset="cellPopover.cellWidth"
flipVerticalOffset="cellPopover.cellHeight"
onMouseWheel="props.onMouseWheel">
onMouseWheel="props.onMouseWheel"
zIndex="zIndex">
<t
t-component="cellPopover.Component"
t-props="{...cellPopover.props, onClosed : () => props.onClosePopover()}"
Expand Down
4 changes: 3 additions & 1 deletion src/components/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ interface Props {
*/
flipVerticalOffset: Pixel;
onMouseWheel: (ev: WheelEvent) => void;
zIndex: Number;
}

export class Popover extends Component<Props, SpreadsheetChildEnv> {
Expand All @@ -37,6 +38,7 @@ export class Popover extends Component<Props, SpreadsheetChildEnv> {
verticalOffset: 0,
marginTop: 0,
onMouseWheel: () => {},
zIndex: ComponentsImportance.Popover,
};

private spreadsheetPosition = useSpreadsheetPosition();
Expand All @@ -55,7 +57,7 @@ export class Popover extends Component<Props, SpreadsheetChildEnv> {
const shadow = maxHeight !== 0 ? "box-shadow: 1px 2px 5px 2px rgb(51 51 51 / 15%);" : "";
return `
position: absolute;
z-index: ${ComponentsImportance.Popover};
z-index: ${this.props.zIndex};
${verticalPosition}px;
${horizontalPosition}px;
${height}px;
Expand Down
5 changes: 3 additions & 2 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,12 +194,13 @@ export enum ComponentsImportance {
Highlight = 5,
Figure = 10,
ScrollBar = 15,
GridPopover = 19,
GridComposer = 20,
Dropdown = 25,
Dropdown = 21,
ColorPicker = 25,
IconPicker = 25,
TopBarComposer = 30,
Popover = 30,
Popover = 35,
ChartAnchor = 1000,
}

Expand Down
2 changes: 1 addition & 1 deletion tests/components/__snapshots__/grid.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@ exports[`error tooltip can display error tooltip 1`] = `
class="o-popover"
style="
position: absolute;
z-index: 30;
z-index: 19;
top:187px;
left:336px;
max-height:960px;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
exports[`link display component simple snapshot 1`] = `
"<div class=\\"o-popover\\" style=\\"
position: absolute;
z-index: 30;
z-index: 19;
top:49px;
left:48px;
max-height:960px;
Expand Down
11 changes: 8 additions & 3 deletions tests/components/spreadsheet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -503,16 +503,21 @@ describe("Composer / selectionInput interactions", () => {
const figureZIndex = getZIndex(".o-figure-wrapper");
const figureAnchorZIndex = getZIndex(".o-fig-anchor");

setCellContent(model, "A1", "=SUM()");
await clickCell(model, "A1");
const gridPopoverZIndex = getZIndex(".o-popover");

expect(gridZIndex).toBeLessThan(highlighZIndex);
expect(highlighZIndex).toBeLessThan(figureZIndex);
expect(figureZIndex).toBeLessThan(vScrollbarZIndex);
expect(vScrollbarZIndex).toEqual(hScrollbarZIndex);
expect(hScrollbarZIndex).toEqual(scrollbarCornerZIndex);
expect(scrollbarCornerZIndex).toBeLessThan(gridComposerZIndex);
expect(scrollbarCornerZIndex).toBeLessThan(gridPopoverZIndex);
expect(gridPopoverZIndex).toBeLessThan(gridComposerZIndex);
expect(gridComposerZIndex).toBeLessThan(dropDownZIndex);
expect(dropDownZIndex).toEqual(colorPickerZIndex);
expect(dropDownZIndex).toBeLessThan(colorPickerZIndex);
expect(colorPickerZIndex).toBeLessThan(topBarComposerZIndex);
expect(topBarComposerZIndex).toEqual(contextMenuZIndex);
expect(topBarComposerZIndex).toBeLessThan(contextMenuZIndex);
expect(contextMenuZIndex).toBeLessThan(figureAnchorZIndex);
});

Expand Down

0 comments on commit 8c89731

Please sign in to comment.