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

added check for secondaryMonitor before accessing members #20

Merged
merged 3 commits into from Mar 17, 2022

Conversation

ja-beb
Copy link
Contributor

@ja-beb ja-beb commented Mar 16, 2022

Fixed issue encountered in Gnome 42 where the secondary monitor was causing an issue on user login. Checking for secondary monitor prior to accessing members fixes the issue.

@ja-beb
Copy link
Contributor Author

ja-beb commented Mar 16, 2022

Screenshot from 2022-03-16 11-38-51

SecondaryMonitorDisplay.prototype._getThumbnailsHeight = function(box) {
if (!this._thumbnails.visible)
return 0;
if (SecondaryMonitorDisplay) {
Copy link
Owner

@axxapy axxapy Mar 16, 2022

Choose a reason for hiding this comment

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

Could you please return instead of making nested code?

Suggested change
if (SecondaryMonitorDisplay) {
if (!SecondaryMonitorDisplay) return; // gnome 42: this object does not exist when there is only one monitor

Thanks

@ja-beb ja-beb requested a review from axxapy March 16, 2022 19:17
Modified to return if SecondaryMonitorDisplay when it is not found. Removed second check (covered by function call).
// Thumbnails on second monitor
if (!SecondaryMonitorDisplay) return;
Copy link
Owner

@axxapy axxapy Mar 16, 2022

Choose a reason for hiding this comment

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

Looks great!

May I also ask you to remove all other changes except for this line? (as no other changes actually change anything).

Thank you

@ja-beb ja-beb requested a review from axxapy March 16, 2022 22:40
@ja-beb
Copy link
Contributor Author

ja-beb commented Mar 16, 2022

Updated: removed my local revisions.

@axxapy axxapy merged commit 802432c into axxapy:master Mar 17, 2022
@axxapy
Copy link
Owner

axxapy commented Mar 17, 2022

Thanks.
Changes are on review, will be published soon: https://extensions.gnome.org/review/30252

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

Successfully merging this pull request may close these issues.

None yet

2 participants