-
-
Notifications
You must be signed in to change notification settings - Fork 113
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
feat: Add welcome screen to seamly2d #1067
Conversation
@@ -297,13 +300,13 @@ void PreferencesConfigurationPage::Apply() | |||
m_defaultExportFormatChanged = false; | |||
} | |||
|
|||
settings->SetOsSeparator(ui->osOptionCheck->isChecked()); | |||
//settings->SetSendReportState(ui->sendReportCheck->isChecked()); |
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.
Hey @DSCaskey - what happened to SetSendReportState ?
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.
The reports went to Valentina if I'm not mistaken. I just disabled the report not knowing if we would want to update it to send to us. Personlly I think it's a waste of time.
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.
Oh... and the reports are only applicable if the build was built with the minGW and DrMingw(). Again... a waste considering the builds are MSVC.
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.
Basically more RT CopyPasta.
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.
It also has security ramifications... just like trying to send a bodyscan file from the app.
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.
Hey slspencer
What do you want me to do with this? As I pointed out the whole reports thing is just baggage code and doesn't even get built. I can remove it here... or preferably remove it all in a seperate issue / PR?
Hey Susan... as I pointed out in several PR comments the reports code is
all useless baggage that doesn't even build when using MSVC. At the time I
commented out the report sending as it was still going to RT.
Question is what do you want me to do right now? Remove all the reporting
code now? Or as I prefer - we leave it as is... merge the PR, and I can
remove the reporting code as a seperate issue ?
…On Tue, Mar 5, 2024, 2:13 PM slspencer ***@***.***> wrote:
***@***.**** requested changes on this pull request.
------------------------------
In src/app/seamly2d/dialogs/configpages/preferencesconfigurationpage.cpp
<#1067 (comment)>
:
> @@ -297,13 +300,13 @@ void PreferencesConfigurationPage::Apply()
m_defaultExportFormatChanged = false;
}
- settings->SetOsSeparator(ui->osOptionCheck->isChecked());
- //settings->SetSendReportState(ui->sendReportCheck->isChecked());
Hey @DSCaskey <https://github.com/DSCaskey> - what happened to
SetSendReportState ?
—
Reply to this email directly, view it on GitHub
<#1067 (review)>,
or unsubscribe
<https://github.com/notifications/unsubscribe-auth/AHTXADXWB7MBKB7RMU3DBZ3YWYKMRAVCNFSM6AAAAABEC3HAZSVHI2DSMVQWIX3LMV43YUDVNRWFEZLROVSXG5CSMV3GSZLXHMYTSMJXHA4DSOJZG4>
.
You are receiving this because you were mentioned.Message ID:
***@***.***>
|
This adds a Welcome screen to Seamly 2D
Allows first time users to set these default settings:
Option to not show Welcome screen again.
Resolves issue #1060
Resolves issue #271