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

Bump some dev dependencies #307

Merged
merged 1 commit into from
Oct 7, 2016
Merged

Conversation

razzeee
Copy link
Contributor

@razzeee razzeee commented Sep 21, 2016

Seems to work fine with Mattermost platform pre release and 3.2

You can find builds here: https://circleci.com/gh/Razzeee/desktop/27#artifacts/containers/0

Copy link
Contributor

@yuya-oc yuya-oc left a comment

Choose a reason for hiding this comment

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

Please rebase and follow my reviews.

"electron-packager": "^7.0.1",
"electron-prebuilt": "1.2.8",
"electron-connect": "~0.6.0",
"electron-packager": "^8.0.0",
Copy link
Contributor

@yuya-oc yuya-oc Sep 22, 2016

Choose a reason for hiding this comment

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

You don't have to bump electron-packager. I'm working on updating build commands.
(In addition, gulpfile.js uses the deprecated parameter, version-string. So this might break artifacts.)

@yuya-oc
Copy link
Contributor

yuya-oc commented Sep 22, 2016

If you write the detail, for example, the motivation to bump, the part which you checked, etc., it's helpful to review.

@yuya-oc
Copy link
Contributor

yuya-oc commented Sep 22, 2016

As you wrote, electron 1.4.0 seems to work with the pre-release server and v3.2.0 server. So I think we can bump electron.

@razzeee
Copy link
Contributor Author

razzeee commented Oct 1, 2016

Updated

@jasonblais
Copy link
Contributor

welcome back @razzeee :)

Copy link
Contributor

@yuya-oc yuya-oc left a comment

Choose a reason for hiding this comment

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

Thanks!

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 3, 2016

@razzeee Just curious, post's background blinks when moving the mouse cursor across the posts. Have you experienced?

@razzeee
Copy link
Contributor Author

razzeee commented Oct 3, 2016

@yuya-oc
They just highlight, as normal for me on windows 10

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 4, 2016

Tested again on Window 10 and macOS.

On Windows 10, I saw the blinking like the gif (actually, it's very fast blinking). @jasonblais and others, would you check this?

background_blinking

On macOS, the app seems to work well.

@jasonblais
Copy link
Contributor

@yuya-oc

Hmm, haven't been able to reproduce the blinking. I tried it on the very same messages you shared above. Does it reproduce consistently across channels and messages?

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 6, 2016

@jasonblais Yes, it happens also on channel list, but messages cause that more often. It seems that the vertical position of mouse cursor is randomly wrong in the window, so I see the blinking.

Then, I tried the app on some DPI. On 100% and 125%, the problem seems not to happen. On 150%(I'm using), it happens. I think almost of users use 100% or 125%, so I feel we might as well merge this for now. Any thoughts?

@jasonblais
Copy link
Contributor

Yeah, I think merging makes sense.

Should we add a known issue for the next changelog that using DPI of 150% might cause the vertical position of mouse cursor to be incorrect? It's okay not to fix it, but let's others know about it.

@yuya-oc
Copy link
Contributor

yuya-oc commented Oct 7, 2016

@jasonblais Sure, that should be written. I add it later.

@yuya-oc yuya-oc added this to the v3.5.0 milestone Oct 7, 2016
@yuya-oc yuya-oc merged commit 4f142f3 into mattermost:master Oct 7, 2016
yuya-oc added a commit that referenced this pull request Oct 7, 2016
@razzeee razzeee deleted the random-bump branch October 7, 2016 15:14
@yuya-oc yuya-oc mentioned this pull request Dec 6, 2016
3 tasks
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.

3 participants