Skip to content

Conversation

crisbeto
Copy link
Member

@crisbeto crisbeto commented Apr 29, 2024

Includes the following fixes related the preview popovers:

fix(cdk/drag-drop): reset user agent color on preview popover

After #28945 the preview is inserted into a popover element which has a user agent styling of color: canvastext. These changes reset it to inherit to match the old behavior.

fix(cdk/drag-drop): remove preview wrapper

Switches back to positioning the preview directly instead of using a wrapper which can be breaking for existing apps. Instead we load a stylesheet dynamically with the necessary resets.

Fixes #28974.

@crisbeto crisbeto added target: patch This PR is targeted for the next patch release merge: preserve commits When the PR is merged, a rebase and merge should be performed labels Apr 29, 2024
@crisbeto crisbeto force-pushed the 28974/drag-drop-popover branch from 7b2b8bb to fd11606 Compare April 29, 2024 07:42
After angular#28945 the preview is inserted into a `popover` element which has a user agent styling of `color: canvastext`. These changes reset it to `inherit` to match the old behavior.

Fixes angular#28974.
@crisbeto crisbeto force-pushed the 28974/drag-drop-popover branch from fd11606 to d99f9f9 Compare April 30, 2024 17:41
@crisbeto crisbeto added the dev-app preview When applied, previews of the dev-app are deployed to Firebase label Apr 30, 2024
Copy link

github-actions bot commented Apr 30, 2024

Deployed dev-app for c356f65 to: https://ng-dev-previews-comp--pr-angular-components-28976-dev-t3fgnong.web.app

Note: As new commits are pushed to this pull request, this link is updated after the preview is rebuilt.

'position': 'fixed',
'top': '0',
'left': '0',
'z-index': this._zIndex + '',
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need z-index if its in a popover?

Copy link
Member Author

@crisbeto crisbeto Apr 30, 2024

Choose a reason for hiding this comment

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

We still have it, because there's a public API to set it and in case we're running on a browser that doesn't support popovers.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think all the browsers we support also support popover, but I guess we can leave it until we deprecate the zindex API

@crisbeto crisbeto force-pushed the 28974/drag-drop-popover branch from d99f9f9 to c356f65 Compare April 30, 2024 18:22
Copy link
Contributor

@mmalerba mmalerba left a comment

Choose a reason for hiding this comment

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

LGTM

Switches back to positioning the preview directly instead of using a wrapper which can be breaking for existing apps. Instead we load a stylesheet dynamically with the necessary resets.
@crisbeto crisbeto removed the dev-app preview When applied, previews of the dev-app are deployed to Firebase label May 1, 2024
@crisbeto crisbeto force-pushed the 28974/drag-drop-popover branch from c356f65 to 53fd236 Compare May 1, 2024 07:35
@crisbeto crisbeto added the action: merge The PR is ready for merge by the caretaker label May 1, 2024
@crisbeto crisbeto merged commit 9729a53 into angular:main May 1, 2024
@angular-automatic-lock-bot
Copy link

This issue has been automatically locked due to inactivity.
Please file a new issue if you are encountering a similar or related problem.

Read more about our automatic conversation locking policy.

This action has been performed automatically by a bot.

@angular-automatic-lock-bot angular-automatic-lock-bot bot locked and limited conversation to collaborators Jun 1, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
action: merge The PR is ready for merge by the caretaker merge: preserve commits When the PR is merged, a rebase and merge should be performed target: patch This PR is targeted for the next patch release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug(cdk/drag-drop): new preview container breaks styling
2 participants