Skip to content

Commit

Permalink
[FIX] popover: adjusted z-index for popovers
Browse files Browse the repository at this point in the history
In these commit three separate issues has been resolved.

1. Fixed an issue where the drop-down menu was appearing behind the
Grid Composer due to a lower z-index. The problem was resolved by increasing
its z-index.

2. Addressed a problem that occurred when both the Grid Composer and TopBar
Composer were active simultaneously, since they shared the same z-index value.
Two enums were created to resolve this issue, with the Grid Composer being given
its own component, and the z-index for the TopBar Composer being set in the
composer.ts file.

3. Solved an issue where Popovers were appearing on top of the Composer
Assistant because they had a higher z-index value. A new enum called GridPopover
was introduced, so that 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 remains the same.

task-3211697

closes #2279

Signed-off-by: Rémi Rahir (rar) <[email protected]>
  • Loading branch information
dhrp-odoo committed Mar 29, 2023
1 parent a6f9300 commit aadde0e
Show file tree
Hide file tree
Showing 10 changed files with 41 additions and 14 deletions.
5 changes: 4 additions & 1 deletion src/components/composer/composer/composer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,6 @@ css/* scss */ `
padding: 0;
margin: 0;
border: 0;
z-index: ${ComponentsImportance.Composer};
flex-grow: 1;
max-height: inherit;
.o-composer {
Expand Down Expand Up @@ -85,6 +84,10 @@ css/* scss */ `
.o-topbar-toolbar .o-composer-container:focus-within {
border: 1px solid ${SELECTION_BORDER_COLOR};
}
.o-topbar-toolbar .o-composer-container {
z-index: ${ComponentsImportance.TopBarComposer};
}
`;

export interface AutocompleteValue {
Expand Down
2 changes: 1 addition & 1 deletion src/components/composer/grid_composer/grid_composer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const SCROLLBAR_HIGHT = 15;
const COMPOSER_BORDER_WIDTH = 3 * 0.4 * window.devicePixelRatio || 1;
css/* scss */ `
div.o-grid-composer {
z-index: ${ComponentsImportance.Composer};
z-index: ${ComponentsImportance.GridComposer};
box-sizing: border-box;
position: absolute;
border: ${COMPOSER_BORDER_WIDTH}px solid ${SELECTION_BORDER_COLOR};
Expand Down
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 { Position, Rect, 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 @@
maxWidth="cellPopover.Component.size and cellPopover.Component.size.maxHidth"
anchorRect="cellPopover.anchorRect"
containerRect="env.getPopoverContainerRect()"
onMouseWheel="props.onMouseWheel">
onMouseWheel="props.onMouseWheel"
zIndex="zIndex">
<t
t-component="cellPopover.Component"
t-props="{...cellPopover.props, onClosed : () => props.onClosePopover()}"
Expand Down
13 changes: 12 additions & 1 deletion src/components/popover/popover.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import { ComponentsImportance } from "../../constants";
import { rectIntersection } from "../../helpers/rectangle";
import { DOMCoordinates, DOMDimension, Pixel, Rect, SpreadsheetChildEnv } from "../../types";
import { PopoverPropsPosition } from "../../types/cell_popovers";
import { css } from "../helpers/css";
import { css, cssPropertiesToCss } from "../helpers/css";
import { usePopoverContainer, useSpreadsheetRect } from "../helpers/position_hook";
import { CSSProperties } from "./../../types/misc";

Expand All @@ -29,6 +29,9 @@ export interface PopoverProps {
onMouseWheel?: () => void;
onPopoverMoved?: () => void;
onPopoverHidden?: () => void;

/** Setting popover to allow dynamic zIndex */
zIndex?: Number;
}

css/* scss */ `
Expand All @@ -50,6 +53,7 @@ export class Popover extends Component<PopoverProps, SpreadsheetChildEnv> {
onMouseWheel: () => {},
onPopoverMoved: () => {},
onPopoverHidden: () => {},
zIndex: ComponentsImportance.Popover,
};

private popoverRef = useRef("popover");
Expand Down Expand Up @@ -103,6 +107,12 @@ export class Popover extends Component<PopoverProps, SpreadsheetChildEnv> {
this.currentPosition = newPosition;
});
}

get popoverStyle(): string {
return cssPropertiesToCss({
"z-index": `${this.props.zIndex}`,
});
}
}

Popover.props = {
Expand All @@ -115,6 +125,7 @@ Popover.props = {
onMouseWheel: { type: Function, optional: true },
onPopoverHidden: { type: Function, optional: true },
onPopoverMoved: { type: Function, optional: true },
zIndex: { type: Number, optional: true },
slots: Object,
};

Expand Down
6 changes: 5 additions & 1 deletion src/components/popover/popover.xml
Original file line number Diff line number Diff line change
@@ -1,7 +1,11 @@
<templates>
<t t-name="o-spreadsheet-Popover" owl="1">
<t t-portal="'.o-spreadsheet'">
<div class="o-popover" t-ref="popover" t-on-wheel="props.onMouseWheel">
<div
class="o-popover"
t-ref="popover"
t-on-wheel="props.onMouseWheel"
t-att-style="popoverStyle">
<t t-slot="default"/>
</div>
</t>
Expand Down
8 changes: 5 additions & 3 deletions src/constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -198,11 +198,13 @@ export enum ComponentsImportance {
Highlight = 5,
Figure = 10,
ScrollBar = 15,
Dropdown = 16,
Composer = 20,
GridPopover = 19,
GridComposer = 20,
Dropdown = 21,
ColorPicker = 25,
IconPicker = 25,
Popover = 30,
TopBarComposer = 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 @@ -120,7 +120,7 @@ height: 1px;
exports[`error tooltip can display error tooltip 1`] = `
<div
class="o-popover"
style="display: block; max-height: 824px; max-width: 697px; top: 187px; left: 336px;"
style="z-index: 19; display: block; max-height: 824px; max-width: 697px; top: 187px; left: 336px;"
>
<div
class="o-error-tooltip"
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
// Jest Snapshot v1, https://goo.gl/fbAQLP

exports[`link display component simple snapshot 1`] = `
"<div class=\\"o-popover\\" style=\\"display: block; max-height: 962px; max-width: 985px; top: 49px; left: 48px;\\"><div class=\\"o-link-tool d-flex align-items-center\\"><!-- t-key to prevent owl from re-using the previous img element when the link changes.
"<div class=\\"o-popover\\" style=\\"z-index: 19; display: block; max-height: 962px; max-width: 985px; top: 49px; left: 48px;\\"><div class=\\"o-link-tool d-flex align-items-center\\"><!-- t-key to prevent owl from re-using the previous img element when the link changes.
The wrong/previous image would be displayed while the new one loads --><img src=\\"https://www.google.com/s2/favicons?sz=16&amp;domain=https://url.com\\"><a class=\\"o-link\\" target=\\"_blank\\" href=\\"https://url.com\\" title=\\"https://url.com\\">https://url.com</a><span class=\\"o-link-icon o-unlink\\" title=\\"Remove link\\"><svg class=\\"o-icon\\" viewBox=\\"0 0 512 512\\"><path fill=\\"currentColor\\" d=\\"M304.083 405.907c4.686 4.686 4.686 12.284 0 16.971l-44.674 44.674c-59.263 59.262-155.693 59.266-214.961 0-59.264-59.265-59.264-155.696 0-214.96l44.675-44.675c4.686-4.686 12.284-4.686 16.971 0l39.598 39.598c4.686 4.686 4.686 12.284 0 16.971l-44.675 44.674c-28.072 28.073-28.072 73.75 0 101.823 28.072 28.072 73.75 28.073 101.824 0l44.674-44.674c4.686-4.686 12.284-4.686 16.971 0l39.597 39.598zm-56.568-260.216c4.686 4.686 12.284 4.686 16.971 0l44.674-44.674c28.072-28.075 73.75-28.073 101.824 0 28.072 28.073 28.072 73.75 0 101.823l-44.675 44.674c-4.686 4.686-4.686 12.284 0 16.971l39.598 39.598c4.686 4.686 12.284 4.686 16.971 0l44.675-44.675c59.265-59.265 59.265-155.695 0-214.96-59.266-59.264-155.695-59.264-214.961 0l-44.674 44.674c-4.686 4.686-4.686 12.284 0 16.971l39.597 39.598zm234.828 359.28l22.627-22.627c9.373-9.373 9.373-24.569 0-33.941L63.598 7.029c-9.373-9.373-24.569-9.373-33.941 0L7.029 29.657c-9.373 9.373-9.373 24.569 0 33.941l441.373 441.373c9.373 9.372 24.569 9.372 33.941 0z\\"></path></svg></span><span class=\\"o-link-icon o-edit-link\\" title=\\"Edit link\\"><svg class=\\"o-icon\\" viewBox=\\"0 0 576 512\\"><path fill=\\"currentColor\\" d=\\"M402.6 83.2l90.2 90.2c3.8 3.8 3.8 10 0 13.8L274.4 405.6l-92.8 10.3c-12.4 1.4-22.9-9.1-21.5-21.5l10.3-92.8L388.8 83.2c3.8-3.8 10-3.8 13.8 0zm162-22.9l-48.8-48.8c-15.2-15.2-39.9-15.2-55.2 0l-35.4 35.4c-3.8 3.8-3.8 10 0 13.8l90.2 90.2c3.8 3.8 10 3.8 13.8 0l35.4-35.4c15.2-15.3 15.2-40 0-55.2zM384 346.2V448H64V128h229.8c3.2 0 6.2-1.3 8.5-3.5l40-40c7.6-7.6 2.2-20.5-8.5-20.5H48C21.5 64 0 85.5 0 112v352c0 26.5 21.5 48 48 48h352c26.5 0 48-21.5 48-48V306.2c0-10.7-12.9-16-20.5-8.5l-40 40c-2.2 2.3-3.5 5.3-3.5 8.5z\\"></path></svg></span></div></div>"
`;
12 changes: 8 additions & 4 deletions tests/components/spreadsheet.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -179,6 +179,9 @@ describe("Simple Spreadsheet Component", () => {
const gridComposerZIndex = getZIndex("div.o-grid-composer");
const highlighZIndex = getZIndex(".o-highlight");

await typeInComposerTopBar("=SUM(A1,A2)");
const topBarComposerZIndex = getZIndex(".o-topbar-toolbar .o-composer-container");

triggerMouseEvent(".o-tool.o-dropdown-button.o-with-color", "click");
await nextTick();
const colorPickerZIndex = getZIndex("div.o-color-picker");
Expand All @@ -194,10 +197,11 @@ describe("Simple Spreadsheet Component", () => {
expect(figureZIndex).toBeLessThan(vScrollbarZIndex);
expect(vScrollbarZIndex).toEqual(hScrollbarZIndex);
expect(hScrollbarZIndex).toEqual(scrollbarCornerZIndex);
expect(scrollbarCornerZIndex).toBeLessThan(dropDownZIndex);
expect(dropDownZIndex).toBeLessThan(gridComposerZIndex);
expect(gridComposerZIndex).toBeLessThan(colorPickerZIndex);
expect(colorPickerZIndex).toBeLessThan(contextMenuZIndex);
expect(scrollbarCornerZIndex).toBeLessThan(gridComposerZIndex);
expect(gridComposerZIndex).toBeLessThan(dropDownZIndex);
expect(dropDownZIndex).toBeLessThan(colorPickerZIndex);
expect(colorPickerZIndex).toBeLessThan(topBarComposerZIndex);
expect(topBarComposerZIndex).toBeLessThan(contextMenuZIndex);
expect(contextMenuZIndex).toBeLessThan(figureAnchorZIndex);
});

Expand Down

0 comments on commit aadde0e

Please sign in to comment.