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

🐛 fixed fields autocomplete not working in popup #17701

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

susannakosic
Copy link

@susannakosic susannakosic commented Aug 11, 2023

closes #16960

  • replaced iframe by div

🤖 Generated by Copilot at d6e1ccb

The pull request updates the PopupModal component and the portal tests to fix some signin issues and improve the test code quality. It replaces the Frame component with a div element for the signin page, and uses the screen object and the popupFrame element for querying DOM elements.

@@ -38,7 +38,7 @@ export const GlobalStyles = `

body {
margin: 0px;
Copy link
Author

Choose a reason for hiding this comment

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

Note for whoever will review this PR.
When testing signin popup, I noticed some font and paragraph changes on the page: headings font looked smaller, and there was a bigger separation between paragraphs (only when popup is open). This was caused by styles injected in the popup which were previously isolated in the iframe and now are embed in the body. I fixed the visual glitch by adding Inter font and removing parragraph bottom margin from global styles, although I am not 100% sure this was the correct approach.

closes TryGhost#16960
- replaced iframe by div for signin page
@@ -291,14 +294,10 @@ export default class PopupModal extends React.Component {
className += ' dev';
}

return (
<div style={Styles.modalContainer}>
<Frame style={frameStyle} title="portal-popup" head={this.renderFrameStyles()} dataTestId='portal-popup-frame'>

Choose a reason for hiding this comment

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

I think iframe is used here to have a style encapsulation. Putting content directly to document may cause some styling issues in some templates. There are many 3rd party templates using portals for signin and their global style definitions can reach this login popup.

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

Successfully merging this pull request may close these issues.

1Password can't autofill Member email address
3 participants