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.

Sign up for GitHub

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

Fixed crash reporting U.X. #1507

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open

Fixed crash reporting U.X. #1507

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
8cd0ca5
Fixed crash reporting U.X. and rephrased toolbar setting to match tab…
Dec 2, 2021
faf77ce
Adjusted comment for clarity.
Dec 2, 2021
e82e6ae
Removed disabling, added warning icon. Has bugs.
Dec 4, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions interface/resources/icons/warning-svgrepo-com.svg
View file
Open in desktop
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
134 changes: 83 additions & 51 deletions interface/resources/qml/controlsUit/CheckBox.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

import QtQuick 2.2
import QtQuick.Controls 2.2 as Original
import QtGraphicalEffects 1.0

import "../stylesUit"

Expand All @@ -30,6 +31,8 @@ Original.CheckBox {
property string labelFontFamily: "Raleway"
property int labelFontSize: 14;
property int labelFontWeight: Font.DemiBold;
property bool isFeatureDisabled: false
property string featureDisabledToolTip;
focusPolicy: Qt.ClickFocus
hoverEnabled: true

Expand All @@ -43,65 +46,94 @@ Original.CheckBox {
}
}

indicator: Item {
width: isFeatureDisabled ? featureDisabledWarning.width : box.width
anchors.left: parent.left

indicator: Rectangle {
id: box
implicitWidth: boxSize
implicitHeight: boxSize
radius: boxRadius
y: parent.height / 2 - height / 2
border.width: 1
border.color: pressed || hovered
? hifi.colors.checkboxCheckedBorder
: (checkBox.isLightColorScheme ? hifi.colors.checkboxLightFinish : hifi.colors.checkboxDarkFinish)

gradient: Gradient {
GradientStop {
position: 0.2
color: pressed || hovered
? (checkBox.isLightColorScheme ? hifi.colors.checkboxChecked : hifi.colors.checkboxLightStart)
: (checkBox.isLightColorScheme ? hifi.colors.checkboxLightStart : hifi.colors.checkboxDarkStart)
}
GradientStop {
position: 1.0
color: pressed || hovered
? (checkBox.isLightColorScheme ? hifi.colors.checkboxChecked : hifi.colors.checkboxLightFinish)
: (checkBox.isLightColorScheme ? hifi.colors.checkboxLightFinish : hifi.colors.checkboxDarkFinish)
}
}
Image {
id: featureDisabledWarning
visible: isFeatureDisabled
source: "/icons/warning-svgrepo-com.svg"

Rectangle {
visible: pressed || hovered
anchors.centerIn: parent
id: innerBox
width: checkSize - 4
height: width
radius: checkRadius
color: hifi.colors.checkboxCheckedBorder
}
anchors.bottom: box.top
anchors.left: parent.left
anchors.bottomMargin: 3
height: isFeatureDisabled ? checkBox.height / 2.5 : 0
width: height

Rectangle {
id: check
width: checkSize
height: checkSize
radius: checkRadius
anchors.centerIn: parent
color: isRedCheck ? hifi.colors.checkboxCheckedRed : hifi.colors.checkboxChecked
border.width: 2
border.color: isRedCheck? hifi.colors.checkboxCheckedBorderRed : hifi.colors.checkboxCheckedBorder
visible: checked && !pressed || !checked && pressed
ToolTip {
id: toolTip
toolTip: checkBox.featureDisabledToolTip
clip: false // Doesn't seem useful.
}

ColorOverlay {
anchors.fill: parent
source: parent
color: "red"
}
}

Rectangle {
id: disabledOverlay
visible: !enabled
width: boxSize
height: boxSize
id: box
implicitWidth: boxSize
implicitHeight: boxSize
anchors.leftMargin: featureDisabledWarning.width / 2
radius: boxRadius
y: checkBox.height / 2 - height / 2
border.width: 1
border.color: hifi.colors.baseGrayHighlight
color: hifi.colors.baseGrayHighlight
opacity: 0.5
border.color: pressed || hovered
? hifi.colors.checkboxCheckedBorder
: (checkBox.isLightColorScheme ? hifi.colors.checkboxLightFinish : hifi.colors.checkboxDarkFinish)

gradient: Gradient {
GradientStop {
position: 0.2
color: pressed || hovered
? (checkBox.isLightColorScheme ? hifi.colors.checkboxChecked : hifi.colors.checkboxLightStart)
: (checkBox.isLightColorScheme ? hifi.colors.checkboxLightStart : hifi.colors.checkboxDarkStart)
}
GradientStop {
position: 1.0
color: pressed || hovered
? (checkBox.isLightColorScheme ? hifi.colors.checkboxChecked : hifi.colors.checkboxLightFinish)
: (checkBox.isLightColorScheme ? hifi.colors.checkboxLightFinish : hifi.colors.checkboxDarkFinish)
}
}

Rectangle {
id: innerBox
width: checkSize - 4
height: width
radius: checkRadius
anchors.centerIn: parent
color: hifi.colors.checkboxCheckedBorder
visible: pressed || hovered
}

Rectangle {
id: check
width: checkSize
height: checkSize
radius: checkRadius
anchors.centerIn: parent
color: isRedCheck ? hifi.colors.checkboxCheckedRed : hifi.colors.checkboxChecked
border.width: 2
border.color: isRedCheck? hifi.colors.checkboxCheckedBorderRed : hifi.colors.checkboxCheckedBorder
visible: checked && !pressed || !checked && pressed
}

Rectangle {
id: disabledOverlay
visible: !enabled
width: boxSize
height: boxSize
radius: boxRadius
border.width: 1
border.color: hifi.colors.baseGrayHighlight
color: hifi.colors.baseGrayHighlight
opacity: 0.5
}
}
}

Expand Down
11 changes: 9 additions & 2 deletions interface/resources/qml/controlsUit/ToolTip.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,12 @@
// Distributed under the Apache License, Version 2.0.
// See the accompanying file LICENSE or http://www.apache.org/licenses/LICENSE-2.0.html
//
// But practically identical to this:

import QtQuick 2.5

Item {
anchors.fill: parent
property string toolTip
property bool showToolTip: false

Expand All @@ -20,7 +22,8 @@ Item {

width: toolTipText.width + 4
height: toolTipText.height + 4
opacity: (toolTip != "" && showToolTip) ? 1 : 0
opacity: (toolTip != "" && showToolTip) ? 1 : 0 // Doesn't work.
//opacity: 1 // Works.
Copy link
Contributor

Choose a reason for hiding this comment

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

Why doesn't it work?

Copy link
Contributor

@JulianGro JulianGro Dec 19, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Not sure if I am looking at this right, but is 1 : 0 a division by zero?

Copy link
Author

Choose a reason for hiding this comment

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

Julian: No, '?' is a ternary operator. https://en.wikipedia.org/wiki/%3F:#C++

Dale: I have no idea. That's why I couldn't get it to work. See my previous comment on this P.R. I tried several things to isolate the issue but had to give up before figuring it out. Sorry.

color: "#ffffaa"
border.color: "#0a0a0a"
Text {
Expand All @@ -42,10 +45,14 @@ Item {
onEntered: showTimer.start()
onExited: { showToolTip = false; showTimer.stop(); }
hoverEnabled: true

onClicked: {
showToolTip = true;
}
}
Timer {
id: showTimer
interval: 250
onTriggered: { showToolTip = true; }
}
}
}
5 changes: 4 additions & 1 deletion interface/resources/qml/dialogs/preferences/CheckBoxPreference.qml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,13 @@ Preference {
height: spacer.height + Math.max(hifi.dimensions.controlLineHeight, checkBox.implicitHeight)
property bool value: false
Component.onCompleted: {
//checkBox.enabled = preference.enabled;
checkBox.checked = preference.value;
checkBox.isFeatureDisabled = preference.functionalityDisabledTooltip;
checkBox.featureDisabledToolTip = preference.tooltip;
value = checkBox.checked;
preference.value = Qt.binding(function(){ return checkBox.checked; });
value = checkBox.checked;
value = checkBox.checked; // Why is this here twice?
}

function save() {
Expand Down
18 changes: 12 additions & 6 deletions interface/src/Menu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -633,12 +633,18 @@ Menu::Menu() {
true,
&UserActivityLogger::getInstance(),
SLOT(disable(bool)));
addCheckableActionToQMenuAndActionHash(networkMenu,
MenuOption::DisableCrashLogger,
0,
true,
&UserActivityLogger::getInstance(),
SLOT(crashMonitorDisable(bool)));
{
auto ual = &UserActivityLogger::getInstance();
addCheckableActionToQMenuAndActionHash(networkMenu,
MenuOption::GenerateAndSubmitCrashReports,
0,
true,
ual,
SLOT(crashMonitorDisable(bool)),
UNSPECIFIED_POSITION,
QString(),
ual->isCrashMonitorEnabled());
}
addActionToQMenuAndActionHash(networkMenu, MenuOption::ShowDSConnectTable, 0,
qApp, SLOT(loadDomainConnectionDialog()));

Expand Down
2 changes: 1 addition & 1 deletion interface/src/Menu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,6 @@ namespace MenuOption {
const QString DeleteAvatarEntitiesBookmark = "Delete Avatar Entities Bookmark";
const QString DeleteBookmark = "Delete Bookmark...";
const QString DisableActivityLogger = "Disable Activity Logger";
const QString DisableCrashLogger = "Disable Crash Reporter";
const QString DisableEyelidAdjustment = "Disable Eyelid Adjustment";
const QString DisableLightEntities = "Disable Light Entities";
const QString DisplayCrashOptions = "Display Crash Options";
Expand Down Expand Up @@ -123,6 +122,7 @@ namespace MenuOption {
const QString FixGaze = "Fix Gaze (no saccade)";
const QString Forward = "Forward";
const QString FrameTimer = "Show Timer";
const QString GenerateAndSubmitCrashReports = "Generate and Submit Crash Reports";
const QString Help = "Help...";
const QString HomeLocation = "Home ";
const QString IncreaseAvatarSize = "Increase Avatar Size";
Expand Down
12 changes: 7 additions & 5 deletions interface/src/main.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -220,11 +220,13 @@ int main(int argc, const char* argv[]) {
}
qDebug() << "UserActivityLogger is enabled:" << ual.isEnabled();

bool isCrashHandlerEnabled = ual.isCrashMonitorEnabled() || parser.isSet(forceCrashReportingOption);
qDebug() << "Crash handler logger is enabled:" << isCrashHandlerEnabled;
if (isCrashHandlerEnabled) {
auto crashHandlerStarted = startCrashHandler(argv[0]);
qDebug() << "Crash handler started:" << crashHandlerStarted;
{
bool isCrashHandlerEnabled = ual.isCrashMonitorEnabled() || parser.isSet(forceCrashReportingOption);
qDebug() << "Crash handler logger is enabled:" << isCrashHandlerEnabled;
if (isCrashHandlerEnabled) {
auto crashHandlerStarted = startCrashHandler(argv[0]);
qDebug() << "Crash handler started:" << crashHandlerStarted;
}
}

const QString& applicationName = getInterfaceSharedMemoryName();
Expand Down
14 changes: 9 additions & 5 deletions interface/src/ui/PreferencesDialog.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -257,11 +257,15 @@ void setupPreferences() {
}

{
auto getter = []()->bool { return !Menu::getInstance()->isOptionChecked(MenuOption::DisableCrashLogger); };
auto setter = [](bool value) { Menu::getInstance()->setIsOptionChecked(MenuOption::DisableCrashLogger, !value); };
preferences->addPreference(new CheckPreference("Privacy", "Send crashes - Vircadia uses information provided by your "
"client to improve the product through crash reports. By allowing Vircadia to collect "
"this information you are helping to improve the product. ", getter, setter));
auto getter = []()->bool { return Menu::getInstance()->isOptionChecked(MenuOption::GenerateAndSubmitCrashReports); };
auto setter = [](bool value) { Menu::getInstance()->setIsOptionChecked(MenuOption::GenerateAndSubmitCrashReports, !value); };
auto preference = new CheckPreference("Privacy", "Send crashes - Vircadia can use information provided by your "
"client to improve the product through crash reports. By allowing developers to collect "
"this information you are helping to improve the product. ", getter, setter);
if (!UserActivityLogger::getInstance().isCrashMonitorEnabled()) {
preference->setFunctionalityDisabledTooltip("Your version of the application was not built with support for crash reporting.");
}
preferences->addPreference(preference);
}

static const QString AVATAR_TUNING { "Avatar Tuning" };
Expand Down
2 changes: 1 addition & 1 deletion libraries/networking/src/UserActivityLogger.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ public slots:
bool isDisabledSettingSet() const { return _disabled.isSet(); }

bool isCrashMonitorEnabled() { return !_crashMonitorDisabled.get(); }
bool isCrashMonitorDisabledSettingSet() const { return _crashMonitorDisabled.isSet(); }
bool isCrashMonitorDisabledSettingSet() const { return _crashMonitorDisabled.isSet(); } // Is this actually used?

void disable(bool disable);
void crashMonitorDisable(bool disable);
Expand Down
13 changes: 13 additions & 0 deletions libraries/shared/src/Preferences.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,9 @@ class AvatarPreference : public BrowsePreference {

class CheckPreference : public BoolPreference {
Q_OBJECT
//Q_PROPERTY(bool enabled READ getEnabled CONSTANT)
Q_PROPERTY(bool indented READ getIndented CONSTANT)
Q_PROPERTY(QString functionalityDisabledTooltip READ getFunctionalityDisabledTooltip CONSTANT)
public:
using Getter = std::function<bool()>;
using Setter = std::function<void(const bool&)>;
Expand All @@ -391,10 +393,21 @@ class CheckPreference : public BoolPreference {
: BoolPreference(category, name, getter, setter) { }
Type getType() override { return Checkbox; }

//bool getEnabled() { return _isEnabled; }
//void setEnabled(const bool enabled) { _isEnabled = enabled; }

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this being added?

Copy link
Author

Choose a reason for hiding this comment

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

Why is this being added?

I was using that for a previous approach and decided to leave it as a comment rather than taking it out, since it might still be useful at some point. You can remove it if you want.

bool getIndented() { return _isIndented; }
void setIndented(const bool indented) { _isIndented = indented; }

//const QString& getTooltip() { return _tooltip; }
//void setTooltip(const QString tooltip) { _tooltip = tooltip; }

const QString& getFunctionalityDisabledTooltip() { return _functionalityDisabledTooltip; }
void setFunctionalityDisabledTooltip(const QString functionalityDisabledTooltip) { _functionalityDisabledTooltip = functionalityDisabledTooltip; }
protected:
//bool _isEnabled { true };
bool _isIndented { false };
QString _functionalityDisabledTooltip;
};

class PrimaryHandPreference : public StringPreference {
Expand Down
7 changes: 5 additions & 2 deletions libraries/ui/src/ui/Menu.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -208,12 +208,14 @@ QAction* Menu::addCheckableActionToQMenuAndActionHash(MenuWrapper* destinationMe
const QObject* receiver,
const char* member,
int menuItemLocation,
const QString& grouping) {
const QString& grouping,
const bool functionalityEnabled) {

QAction* action = addActionToQMenuAndActionHash(destinationMenu, actionName, shortcut, receiver, member,
QAction::NoRole, menuItemLocation);
action->setCheckable(true);
action->setChecked(checked);
action->setDisabled(!functionalityEnabled);

if (isValidGrouping(grouping)) {
_groupingActions[grouping] << action;
Expand All @@ -229,7 +231,8 @@ QAction* Menu::addCheckableActionToQMenuAndActionHash(MenuWrapper* destinationMe
const QKeySequence& shortcut,
const bool checked,
int menuItemLocation,
const QString& grouping) {
const QString& grouping) {

auto action = addCheckableActionToQMenuAndActionHash(destinationMenu, actionName, shortcut, checked, nullptr, nullptr, menuItemLocation, grouping);
connect(action, &QAction::triggered, handler);
return action;
Expand Down
3 changes: 2 additions & 1 deletion libraries/ui/src/ui/Menu.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,8 @@ class Menu : public QMenuBar {
const QObject* receiver = NULL,
const char* member = NULL,
int menuItemLocation = UNSPECIFIED_POSITION,
const QString& grouping = QString());
const QString& grouping = QString(),
const bool functionalityEnabled = true);

QAction* addCheckableActionToQMenuAndActionHash(MenuWrapper* destinationMenu,
const QString& actionName,
Expand Down