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

Fields with buttons look incorrectly after focusing button #1917

Closed
charlag opened this issue Mar 18, 2020 · 6 comments · Fixed by #2134
Closed

Fields with buttons look incorrectly after focusing button #1917

charlag opened this issue Mar 18, 2020 · 6 comments · Fixed by #2134
Assignees
Labels
browser-bug Bugs caused by environment desktop Desktop client related issues state:tested We tested it and are about to release it
Milestone

Comments

@charlag
Copy link
Contributor

charlag commented Mar 18, 2020

I think there was some CSS engine change in Blink, does not happen in Firefox eg.

2_texts_nok

@charlag charlag added the bug broken functionality, usability problems, unexpected errors label Mar 18, 2020
@mpfau
Copy link
Contributor

mpfau commented Mar 19, 2020

This is a problem with the negative margin that we apply to the buttons container to right align the buttons correctly.

This seems to be triggered only by negative margins with flex layouts.

@mpfau mpfau added this to the Next release milestone Mar 19, 2020
@charlag charlag self-assigned this Mar 19, 2020
charlag added a commit that referenced this issue Mar 19, 2020
Changes in Blink caused our text fields with buttons to look incorrectly
after focusing one of the buttons. This happened because we used
negative margin to align button to the right edge (to the text field
and other component).

Instead we now align button contents to the right in text fields.
There are still couple of places where negative margin is used but
there are no issues with it so far.

This commit changes old-style DropdownSelector to use new-style buttons
internally to avoid implementing endAligned for the old-style Button.

This commit also changes buttons to the new ones whenever possible.
charlag added a commit that referenced this issue Mar 19, 2020
Changes in Blink caused our text fields with buttons to look incorrectly
after focusing one of the buttons. This happened because we used
negative margin to align button to the right edge (to the text field
and other component).

Instead we now align button contents to the right in text fields.
There are still couple of places where negative margin is used but
there are no issues with it so far.

This commit changes old-style DropdownSelector to use new-style buttons
internally to avoid implementing endAligned for the old-style Button.

This commit also changes buttons to the new ones whenever possible.
charlag added a commit that referenced this issue Mar 19, 2020
Changes in Blink caused our text fields with buttons to look incorrectly
after focusing one of the buttons. This happened because we used
negative margin to align button to the right edge (to the text field
and other component).

Instead we now align button contents to the right in text fields.
There are still couple of places where negative margin is used but
there are no issues with it so far.

This commit changes old-style DropdownSelector to use new-style buttons
internally to avoid implementing endAligned for the old-style Button.

This commit also changes buttons to the new ones whenever possible.
@bedhub bedhub modified the milestones: 3.69.x, Next Release Mar 26, 2020
@charlag
Copy link
Contributor Author

charlag commented Mar 26, 2020

Seems like it's rolled back/fixed in Chrome Version 82.0.4085.1 (Dev) but still happens in 80.0.3987.149

@charlag charlag linked a pull request Mar 31, 2020 that will close this issue
@charlag charlag added browser-bug Bugs caused by environment and removed bug broken functionality, usability problems, unexpected errors labels Mar 31, 2020
@bedhub bedhub modified the milestones: Next Release, Next next release Apr 8, 2020
@bedhub bedhub modified the milestones: 3.71.5, Next next release Apr 16, 2020
@bedhub bedhub modified the milestones: 3.72.1, Next Next Release Apr 27, 2020
@bedhub
Copy link
Contributor

bedhub commented Apr 27, 2020

We will not fix it but keep this issue open until the fix has been released in stable chrome.

@bedhub bedhub added the state:wontfix issues that are not significant enough to invest in or that are intended behaviour label Apr 27, 2020
@bedhub
Copy link
Contributor

bedhub commented May 29, 2020

Check if an update of electron fixes this issue

@bedhub bedhub modified the milestones: 3.74.x, Next release May 29, 2020
@bedhub bedhub added desktop Desktop client related issues and removed state:wontfix issues that are not significant enough to invest in or that are intended behaviour labels May 29, 2020
@bedhub bedhub assigned ganthern and unassigned charlag May 29, 2020
@ganthern ganthern linked a pull request Jun 4, 2020 that will close this issue
@ganthern
Copy link
Contributor

ganthern commented Jun 4, 2020

still occurs in Electron 8.3.1 (Chromium 80.0.3987.165), next Electron version bump to 9.x.y should fix it (Chromium 83)

@bedhub bedhub modified the milestones: 3.75.x, Next release Jun 5, 2020
ganthern added a commit that referenced this issue Jul 2, 2020
electron 9 uses Chromium 83, which fixes the negative margin bug on
fields with buttons.

e9 changes electron.shell.openItem(path):boolean to
electron.shell.openPath(path):Promise<string>
@ganthern ganthern linked a pull request Jul 2, 2020 that will close this issue
@bedhub bedhub modified the milestones: Next release, 3.75.1 Jul 10, 2020
@charlag
Copy link
Contributor Author

charlag commented Jul 13, 2020

Test notes: check downloads and opening files on all platforms

@bedhub bedhub assigned bedhub and unassigned ganthern Jul 15, 2020
@bedhub bedhub added the state:tested We tested it and are about to release it label Jul 15, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
browser-bug Bugs caused by environment desktop Desktop client related issues state:tested We tested it and are about to release it
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants