-
-
Notifications
You must be signed in to change notification settings - Fork 875
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
Make expert mode permanent even after manually switching it off and reconnecting #2866
Conversation
This comment has been minimized.
This comment has been minimized.
src/js/serial_backend.js
Outdated
// reset expert mode | ||
ConfigStorage.get('permanentExpertMode', function (result) { | ||
const checked = result.permanentExpertMode; | ||
$('input[name="expertModeCheckbox"]').prop('checked', checked).change(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should use the new syntax:
$('input[name="expertModeCheckbox"]').prop('checked', checked).change(); | |
$('input[name="expertModeCheckbox"]').prop('checked', checked).trigger('change'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yea, didn't know that
Done 👍
93d38b6
to
a46ff4b
Compare
Kudos, SonarCloud Quality Gate passed!
|
Do you want to test this code? Here you have an automated build: |
tested on latest build - works as requested - Bug/Request fixed |
ConfigStorage.get('permanentExpertMode', function (result) { | ||
const checked = result.permanentExpertMode; | ||
$('input[name="expertModeCheckbox"]').prop('checked', checked).trigger('change'); | ||
}); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is not need to use callback syntax now (we should drop that completely...)
ConfigStorage.get('permanentExpertMode', function (result) { | |
const checked = result.permanentExpertMode; | |
$('input[name="expertModeCheckbox"]').prop('checked', checked).trigger('change'); | |
}); | |
const result = ConfigStorage.get('permanentExpertMode'); | |
const checked = result.permanentExpertMode; | |
$('input[name="expertModeCheckbox"]').prop('checked', checked).trigger('change'); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@chmelevskij will be fixed in #2636
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, though that one is in already.... soz
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But wait a minute, master already supports sync access, so wouldn't it be better to just go with the correct usage?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
AUTOMERGE: (PASS)
|
I don't see how, I think this one introduced the bug (at least on windows). On presets tab save/load backup buttons don't act normal. At the end they need to restart the configurator page and FC, but they don't anymore. I can check what's happening only in a few days. Now I just tested nightly builds to see where the issue started: e62ab2d |
Hey.
This PR Fixes #2610
If you see any other possibility of implementing this please let me know and lets talk about it.
This is my first commit to this codebase so if I missed anything process related let me know.