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

Updated NW.js to 0.54.0. #2507

Merged
merged 1 commit into from Jun 21, 2021
Merged

Conversation

mikeller
Copy link
Member

@mikeller mikeller commented Jun 9, 2021

@mikeller mikeller mentioned this pull request Jun 9, 2021
@sonarcloud
Copy link

sonarcloud bot commented Jun 9, 2021

Kudos, SonarCloud Quality Gate passed!

Bug A 0 Bugs
Vulnerability A 0 Vulnerabilities
Security Hotspot A 0 Security Hotspots
Code Smell A 0 Code Smells

No Coverage information No Coverage information
0.9% 0.9% Duplication

@asizon
Copy link
Member

asizon commented Jun 9, 2021

@mikeller tested here on Windows10 but some weird is happens with report dialog size:

image

@asizon
Copy link
Member

asizon commented Jun 9, 2021

Maybe we just need a small css fix on dialog "height" value to "fit-content"??not sure how this chrome update is afecting to other things.

@haslinghuis
Copy link
Member

haslinghuis commented Jun 9, 2021

Same on Linux. Otherwise most things so far tested seem to just work. Adding following snippet at line 2291 in main.css would fix it:

    #dialogReportProblems,
    #dialogResetToCustomDefaults { 
        height: fit-content;
    }

Or even better change line 1484 from auto to fit-content:

        height: fix-content;

There could be more dialogs hidden so far needing this fix.

@asizon
Copy link
Member

asizon commented Jun 9, 2021

I think I got the cause, there are new style changes aplied to this dialog ID with the "user agent stylesheet"

image

@asizon asizon added this to the 10.8.0 milestone Jun 12, 2021
@haslinghuis haslinghuis self-assigned this Jun 20, 2021
@haslinghuis
Copy link
Member

@asizon how do you prefer to fix this? Already suggested two ways, as you showed another fix? Want to create a PR to fix this issue so this PR can go forward.

@asizon
Copy link
Member

asizon commented Jun 20, 2021

I have share this issue with @McGiverGim lets see what is happens here.

@McGiverGim
Copy link
Member

I prefer to fix it here, in the PR, but if mikeller can't do it now and you have a PR ready with the fix, I don't see a problem to merge this.

Copy link
Member

@asizon asizon left a comment

Choose a reason for hiding this comment

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

Ok so LGTM, @haslinghuis then lets aply height: fit-content in the .dialog class in next PR

@blckmn
Copy link
Member

blckmn commented Jun 21, 2021

AUTOMERGE: (PASS)

  • Github identifies PR as mergeable -> PASS
  • Assigned to a milestone -> PASS
  • cooling off period lapsed -> PASS
  • commit count less or equal to three -> PASS
  • 'Don't merge' label NOT found -> PASS
  • At least one 'RN:' label found -> PASS
  • 'Tested' label found -> PASS
  • assigned to an approver -> PASS
  • approver count at least three -> PASS

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.

None yet

5 participants