Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[FIX] popover: adjusted z-index for popover and grid composer #2156

Closed
wants to merge 1 commit into from

Conversation

dhrp-odoo
Copy link
Contributor

@dhrp-odoo dhrp-odoo commented Mar 3, 2023

Description:

Currently error tooltip is overlapping composer assistant when one tries to
modify arguments within the grid composer. It arises because of a conflict in
the z-index values between the popover and grid composer components.

To resolve this problem, the z-index of the popover component was reduced.
Now, the popover is behind the grid composer and allows user to view composer
assistant properly.

Odoo task ID : 3211697

review checklist

  • feature is organized in plugin, or UI components
  • support of duplicate sheet (deep copy)
  • in model/core: ranges are Range object, and can be adapted (adaptRanges)
  • in model/UI: ranges are strings (to show the user)
  • undo-able commands (uses this.history.update)
  • multiuser-able commands (has inverse commands and transformations where needed)
  • new/updated/removed commands are documented
  • exportable in excel
  • translations (_lt("qmsdf %s", abc))
  • unit tested
  • clean commented code
  • track breaking changes
  • doc is rebuild (npm run doc)
  • status is correct in Odoo

@robodoo
Copy link
Collaborator

robodoo commented Mar 3, 2023

Copy link
Contributor

@hokolomopo hokolomopo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

@@ -7,7 +7,7 @@ const { xml } = tags;

const TEMPLATE = xml/* xml */ `
<Portal target="'.o-spreadsheet'">
<div t-att-style="style">
<div t-att-style="style" t-attf-class="o-popover">
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
<div t-att-style="style" t-attf-class="o-popover">
<div class="o-popover" t-att-style="style">

useless and harder to read to use a t-attf for something static

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Changes made

@@ -15,6 +15,10 @@ const CSS = css/* scss */ `
border-left: 3px solid red;
padding: 10px;
}

.o-popover:has(.o-error-tooltip) {
z-index: 4 !important;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note for forward port: should be in enum ComponentImportance after 16.0.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am aware of this and already did the research of how i can achieve the same in 16.0 and above.

Currently error tooltip is overlapping composer assistant when one tries to
modify arguments within the grid composer.  It arises because of a conflict in
the z-index values between the popover and grid composer components.

To resolve this problem, the z-index of the popover component was reduced.
Now, the popover is behind the grid composer and allows user to view composer
assistant properly .

task-3211697
@dhrp-odoo dhrp-odoo force-pushed the 15.0-fix-popover-z-index-dhrp branch from 6537c21 to 0138dc3 Compare March 6, 2023 11:53
@hokolomopo
Copy link
Contributor

small note: when creating the PR, you should update the "TASK_ID" in both the link label and the link itself, otherwise it's just a dead link 😛

@rrahir
Copy link
Collaborator

rrahir commented Mar 9, 2023

@robodoo r+

robodoo pushed a commit that referenced this pull request Mar 9, 2023
Currently error tooltip is overlapping composer assistant when one tries to
modify arguments within the grid composer.  It arises because of a conflict in
the z-index values between the popover and grid composer components.

To resolve this problem, the z-index of the popover component was reduced.
Now, the popover is behind the grid composer and allows user to view composer
assistant properly .

task-3211697

closes #2156

Signed-off-by: Rémi Rahir (rar) <rar@odoo.com>
@robodoo robodoo temporarily deployed to merge March 9, 2023 13:51 Inactive
@robodoo robodoo closed this Mar 9, 2023
@fw-bot
Copy link
Collaborator

fw-bot commented Mar 13, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

7 similar comments
@fw-bot
Copy link
Collaborator

fw-bot commented Mar 14, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

@fw-bot
Copy link
Collaborator

fw-bot commented Mar 15, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

@fw-bot
Copy link
Collaborator

fw-bot commented Mar 16, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

@fw-bot
Copy link
Collaborator

fw-bot commented Mar 17, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

@fw-bot
Copy link
Collaborator

fw-bot commented Mar 18, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

@fw-bot
Copy link
Collaborator

fw-bot commented Mar 19, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

@fw-bot
Copy link
Collaborator

fw-bot commented Mar 21, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2205

@fw-bot fw-bot deleted the 15.0-fix-popover-z-index-dhrp branch March 23, 2023 14:47
@fw-bot
Copy link
Collaborator

fw-bot commented Mar 29, 2023

@dhrp-odoo @rrahir this pull request has forward-port PRs awaiting action (not merged or closed):
#2279

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants