Skip to content
This repository was archived by the owner on Mar 3, 2023. It is now read-only.

Conversation

thomasjo
Copy link
Contributor

@thomasjo thomasjo commented Sep 15, 2016

Reported Issues

Reported issues resolved by #13880:


During my initial testing on macOS 10.12, this seems to be working. Haven't noticed any regressions.
Would be great if more folks could give this a spin 🙇

/cc @atom/maintainers


Fixes #11692
Resolves #12690
Fixes #8006
Fixes #12832
Fixes #13767
Fixes #4084 (Will be fixed by the editor rendering rewrite)
Fixes #13265 (Will be fixed by the editor rendering rewrite)

@thomasjo thomasjo changed the title Upgrade Electron to v1.4.0 Upgrade Electron to v1.4.1 Sep 22, 2016
@thomasjo
Copy link
Contributor Author

Since Electron v1.4.1 ships with electron/electron#7209, I hope this PR will resolve #11692.

@thomasjo thomasjo force-pushed the tj-upgrade-electron branch 2 times, most recently from 4549b73 to 0b54c9d Compare September 25, 2016 14:14
@thomasjo thomasjo force-pushed the tj-upgrade-electron branch from 0b54c9d to fcddf83 Compare October 1, 2016 10:22
@winstliu
Copy link
Contributor

winstliu commented Oct 2, 2016

Observations from around 10 minutes of light usage:

NOTHING IS BLURRY ANYMORE!

I have beautiful subpixel antialiasing on everything, even the actual code for the first time ever* 💥. Fixes #12652, maybe #7261, #3869, #2833.

Scrollbars seem to disappear randomly. Haven't narrowed this down but I did notice one of my files didn't have a scrollbar.

* Just noticed that the tabs and mini-editors don't have subpixel AA, but that's super minor compared to before.

@winstliu
Copy link
Contributor

winstliu commented Oct 2, 2016

Ok, something else related to subpixel AA:
If I scroll all the way to the end of the file or use the scrollbar, I lose subpixel AA in the editor until I switch tabs.

@Ben3eeE
Copy link
Contributor

Ben3eeE commented Oct 3, 2016

Scrollbars seem to disappear randomly. Haven't narrowed this down but I did notice one of my files didn't have a scrollbar.

Have this happening with the vertical scrollbar on the TextEditor active when restoring session. In the case of split panes (I tried left/right) both TextEditors are missing vertical scrollbars.

  1. Open Atom 1.12-dev-fcddf83 open some file that has content enough to have a vertical scrollbar.
  2. Close Atom.
  3. Open Atom 1.12-dev-fcddf83 and keep an eye on the vertical scrollbar. It appears quickly when opening the previous session and then disappears.

If you resize the pane so that the horizontal scrollbar appears or disappears then you can get the vertical scrollbar to appear, but in all other cases this file seems to be missing the vertical scrollbar.

Tested with both Atom Light and One Dark themes. This does not reproduce in 1.11-beta5. Don't know about other dev versions.

@winstliu
Copy link
Contributor

winstliu commented Oct 3, 2016

The disappearing scrollbar issue appears to happen on the first file that is opened/restored. From the dev tools, the scrollbar (and scrollbar corner) is being given a width of 10px instead of the usual 15px. If I change it to 15, then the scrollbar appears. Anything > 10 works. /cc @simurai
I've also been seeing a related issue where the scrollbar flickers like crazy when moving the mouse between the tab bar and the editor. I'll try to find a stable repro case for that, but it also happens on the first file.

Now, some more subpixel antialiasing updates. As I said before, it works for the most part. Here's when it fails:

  1. In a mini-editor
  2. (Almost) anywhere with rendered text (eg Settings View, Tabs, Notifications, About). The tree view and status bar are fine though?
  3. Upon reaching the end of the file
  4. Upon scrolling with a touchpad or scrollbar. Mousewheel scrolling is fine.
  5. Upon editing the last three visible lines, causing the editor to scroll and triggering case 4).
  6. Dev tools

Also, Atom occasionally crashes when I try to close it. I'm not sure if that is specific to the Electron upgrade though as it's happened before as well.

@winstliu
Copy link
Contributor

winstliu commented Oct 4, 2016

More comments.

I appear to lose subpixel antialiasing during any type of scrolling (programmatically through go-to-line or find-and-replace, for example, or manually) except for mousewheel scrolling.

The dev tools are frozen after reloading the window and must be closed through the keybinding and reopened in order for it to work.

@damieng
Copy link
Contributor

damieng commented Oct 4, 2016

Chrome tends to switch from sub-pixel to regular AA when it expects to need to redraw it several times such as in animation. This is because AA can be cached and re-used whereas sub-pixel depends on the horizontal offset.

@simurai
Copy link
Contributor

simurai commented Oct 4, 2016

The scrollbar issue is super weird. Changing the width to anything other than the original (10px) works.. but when changing bottom it alternates. See the sub-pixel (0.1px) steps.

scrollbars

It seems any property that triggers a layout reflow makes the scrollbar visibility alternate when changing the value:

scrollbars

And just typing in search + replace alternates the horizontal scrollbar:

scrollbars

ps. on macOS, this issue only happens if the "Show scroll bars" is set to "Always". Otherwise (where the scroll bars hide automatically) it's fine.

@simurai
Copy link
Contributor

simurai commented Oct 6, 2016

Another thing I noticed. The native tabs in macOS Sierra seem to work in this Electron version. Problem with the custom title bar is that the native tabs are just on top and don't push the rest down. Also, hard to read with dark themes.

screen shot 2016-10-06 at 4 06 06 pm

Native title bar works fine.

Steps to reproduce:

  1. Enable Atom Settings > Core > Use Custom Title Bar.
  2. Switch macOS Prefereces > Dock > "Prefer tabs when opening documents" to "Always":
    screen shot 2016-10-06 at 4 02 36 pm
  3. Open another window with cmd-o or View > Show Tab Bar.

Possible solution

a. Either disable native tabs when custom title bar is enabled.
b. See if it's possible to detect when native tabs get added and then push the content down.

Maybe not that urgent, since you have to enable two things to run into this. Could also be a separate issue.

Update:

Fixed with Electron v1.4.2.

@thomasjo thomasjo force-pushed the tj-upgrade-electron branch from fcddf83 to 2612447 Compare October 6, 2016 07:23
@thomasjo
Copy link
Contributor Author

thomasjo commented Oct 6, 2016

@simurai Electron v1.4.2 disabled the native tabs support, so this shouldn't be an issue anymore 🎱

@thomasjo thomasjo force-pushed the tj-upgrade-electron branch from 2612447 to 9df5253 Compare October 7, 2016 05:31
@Ben3eeE
Copy link
Contributor

Ben3eeE commented Oct 7, 2016

🙈 Drag image 🙈

dragimage

@thomasjo thomasjo force-pushed the tj-upgrade-electron branch 5 times, most recently from f24e7e2 to 8bd2d16 Compare October 12, 2016 06:16
@v3ss0n
Copy link

v3ss0n commented Oct 12, 2016

electron 1.x is coming in atom 1.12?

Antonio Scandurra added 3 commits May 19, 2017 10:23
Adding a source map for the entire snapshot was expensive in terms of
memory and seemed to be triggering some sort of bug in Chromium when
reloading with the DevTools open.

The custom row translation relies on a much more compact representation
of the data and avoids the crash.

Signed-off-by: Nathan Sobo <nathan@github.com>
Signed-off-by: Nathan Sobo <nathan@github.com>
@as-cii
Copy link
Contributor

as-cii commented May 19, 2017

Seems like AppVeyor failed but I believe that's just spurious. I will go ahead and merge it into master so that it can bake for a while before putting it on beta.

Thanks everyone for the help and the hard work on this! 🙏

@as-cii as-cii merged commit 910fbee into master May 19, 2017
facebook-github-bot pushed a commit to facebookarchive/nuclide that referenced this pull request May 31, 2017
Summary:
Imported from joefitzgerald's #1171

In atom/atom#12696, significant changes were made to the composition of the text-editor-component. These changes break Nuclide when working against current Atom master.

This PR allows Hyperclick to correctly function with the text-editor-component changes. It also fixes an issue with logic used to suppress cursor blinking for read only text editors.

Fixes #1154

Reviewed By: hansonw

Differential Revision:
D5151519

Tags: nuclide

fbshipit-source-id: fa7030921260474fd315cb70efc7c5e8384848b3
@thomasjo thomasjo deleted the tj-upgrade-electron branch June 16, 2017 20:43
hansonw pushed a commit to facebookarchive/atom-ide-ui that referenced this pull request Sep 6, 2017
Summary:
Imported from joefitzgerald's facebookarchive/nuclide#1171

In atom/atom#12696, significant changes were made to the composition of the text-editor-component. These changes break Nuclide when working against current Atom master.

This PR allows Hyperclick to correctly function with the text-editor-component changes. It also fixes an issue with logic used to suppress cursor blinking for read only text editors.

Fixes #1154

Reviewed By: hansonw

Differential Revision:
D5151519

Tags: nuclide

fbshipit-source-id: fa7030921260474fd315cb70efc7c5e8384848b3
@jasonrudolph jasonrudolph mentioned this pull request May 8, 2018
60 tasks
@daviwil daviwil mentioned this pull request Dec 20, 2018
59 tasks
@as-cii as-cii mentioned this pull request May 20, 2019
8 tasks
@rafeca rafeca mentioned this pull request Jun 25, 2019
7 tasks
@lkashef lkashef mentioned this pull request May 20, 2020
@lkashef lkashef mentioned this pull request Jul 16, 2020
76 tasks
@sadick254 sadick254 mentioned this pull request Dec 1, 2020
58 tasks
@sadick254 sadick254 mentioned this pull request Aug 19, 2021
64 tasks
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.