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

Bug: MessageBoxIconType.NoSymbol shows Success icon instead of no symbol #2998

Merged
merged 3 commits into from
Jan 10, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions common/api/appui-abstract.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -1367,6 +1367,8 @@ export enum MessageSeverity {
// (undocumented)
Question = 2,
// (undocumented)
Success = 6,
// (undocumented)
Warning = 3
}

Expand Down
4 changes: 4 additions & 0 deletions common/api/core-frontend.api.md
Original file line number Diff line number Diff line change
Expand Up @@ -6515,6 +6515,8 @@ export enum MessageBoxIconType {
// (undocumented)
Question = 2,
// (undocumented)
Success = 5,
// (undocumented)
Warning = 3
}

Expand Down Expand Up @@ -7191,6 +7193,8 @@ export enum OutputMessagePriority {
// (undocumented)
None = 0,
// (undocumented)
Success = 1,
// (undocumented)
Warning = 11
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/appui-abstract",
"comment": "Fix bug that sets the icon on MessageBox.NoSymbol the Success icon.",
"type": "none"
}
],
"packageName": "@itwin/appui-abstract"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/appui-react",
"comment": "Fix bug that sets the icon on MessageBox.NoSymbol the Success icon.",
"type": "none"
}
],
"packageName": "@itwin/appui-react"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-frontend",
"comment": "Fix bug that sets the icon on MessageBox.NoSymbol the Success icon.",
"type": "none"
}
],
"packageName": "@itwin/core-frontend"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
{
"changes": [
{
"packageName": "@itwin/core-react",
"comment": "Fix bug that sets the icon on MessageBox.NoSymbol the Success icon.",
"type": "none"
}
],
"packageName": "@itwin/core-react"
}
2 changes: 2 additions & 0 deletions core/frontend/src/NotificationManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ export enum OutputMessageType {
*/
export enum OutputMessagePriority {
None = 0,
Success = 1,
Error = 10,
Warning = 11,
Info = 12,
Expand Down Expand Up @@ -79,6 +80,7 @@ export enum MessageBoxIconType {
Question = 2, // Question Mark
Warning = 3, // Exclamation Point
Critical = 4, // Stop Sign
Success = 5, // check mark
}

/** Describes the possible return values produced when the user clicks a button in a messagebox opened using [[NotificationManager.openMessageBox]].
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -423,7 +423,7 @@ export class Frontstage4 extends FrontstageProvider {
labelKey="SampleApp:buttons.toolGroup"
iconSpec="icon-placeholder"
items={[
AppTools.successMessageBoxCommand, AppTools.informationMessageBoxCommand, AppTools.questionMessageBoxCommand,
AppTools.noIconMessageBoxCommand, AppTools.successMessageBoxCommand, AppTools.informationMessageBoxCommand, AppTools.questionMessageBoxCommand,
AppTools.warningMessageBoxCommand, AppTools.errorMessageBoxCommand, AppTools.openMessageBoxCommand, AppTools.openMessageBoxCommand2,
this._spinnerTestDialogItem,
this._sampleModelessDialogItem,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -646,13 +646,20 @@ export class AppTools {
execute: () => ModalDialogManager.openDialog(AppTools._messageBox(MessageSeverity.Error, IModelApp.localization.getLocalizedString("SampleApp:buttons.errorMessageBox"))),
});
}
public static get noIconMessageBoxCommand() {
return new CommandItemDef({
commandId: "noIconMessage",
labelKey: "SampleApp:buttons.noIconMessageBox",
execute: () => ModalDialogManager.openDialog(AppTools._messageBox(MessageSeverity.None, IModelApp.localization.getLocalizedString("SampleApp:buttons.noIconMessageBox"))),
});
}

public static get successMessageBoxCommand() {
return new CommandItemDef({
commandId: "successMessage",
iconSpec: "icon-status-success",
labelKey: "SampleApp:buttons.successMessageBox",
execute: () => ModalDialogManager.openDialog(AppTools._messageBox(MessageSeverity.None, IModelApp.localization.getLocalizedString("SampleApp:buttons.successMessageBox"))),
execute: () => ModalDialogManager.openDialog(AppTools._messageBox(MessageSeverity.Success, IModelApp.localization.getLocalizedString("SampleApp:buttons.successMessageBox"))),
});
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,4 +16,5 @@ export enum MessageSeverity {
Warning = 3,
Error = 4,
Fatal = 5,
Success = 6,
}
12 changes: 2 additions & 10 deletions ui/appui-react/src/appui-react/UiFramework.ts
Original file line number Diff line number Diff line change
Expand Up @@ -509,7 +509,7 @@ export class UiFramework {
}

public static get showWidgetIcon(): boolean {
return UiFramework.frameworkState ? UiFramework.frameworkState.configurableUiState.showWidgetIcon : false;
return UiFramework.frameworkState ? UiFramework.frameworkState.configurableUiState.showWidgetIcon : /* istanbul ignore next */ false;
}

public static setShowWidgetIcon(value: boolean) {
Expand All @@ -531,7 +531,7 @@ export class UiFramework {
* @public
*/
public static get viewOverlayDisplay() {
return UiFramework.frameworkState ? UiFramework.frameworkState.configurableUiState.viewOverlayDisplay : true;
return UiFramework.frameworkState ? UiFramework.frameworkState.configurableUiState.viewOverlayDisplay : /* istanbul ignore next */ true;
}
/** Set the variable that controls display of the view overlay. Applies to all viewports in the app
* @public
Expand Down Expand Up @@ -561,14 +561,6 @@ export class UiFramework {
await IModelApp.telemetry.postTelemetry(activity, telemetryEvent);
} catch { }
}
private static _handleFrameworkVersionChangedEvent = (args: FrameworkVersionChangedEventArgs) => {
// Log Ui Version used
Logger.logInfo(UiFramework.loggerCategory(UiFramework), `Ui Version changed to ${args.version} `);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
UiFramework.postTelemetry(`Ui Version changed to ${args.version} `, "F2772C81-962D-4755-807C-2D675A5FF399");
UiFramework.setUiVersion(args.version);
};

/** Determines whether a ContextMenu is open
* @alpha
* */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,6 +89,9 @@ export class StandardMessageBox extends React.PureComponent<StandardMessageBoxPr
case MessageBoxIconType.Critical:
severity = MessageSeverity.Error;
break;
case MessageBoxIconType.Success:
severity = MessageSeverity.Success;
break;
}

return (
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,10 @@ export class MessageManager {

switch (details.priority) {
case OutputMessagePriority.None:
severity = MessageSeverity.None;
severity = MessageSeverity.Success;
break;
case OutputMessagePriority.Success:
severity = MessageSeverity.Success;
break;
case OutputMessagePriority.Info:
severity = MessageSeverity.Information;
Expand All @@ -399,6 +402,9 @@ export class MessageManager {
case OutputMessagePriority.None:
iconType = MessageBoxIconType.NoSymbol;
break;
case OutputMessagePriority.Success:
iconType = MessageBoxIconType.Success;
break;
case OutputMessagePriority.Info:
iconType = MessageBoxIconType.Information;
break;
Expand Down
4 changes: 4 additions & 0 deletions ui/appui-react/src/test/UiFramework.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,10 @@ describe("UiFramework localStorage Wrapper", () => {
const displayOverlay = false;
UiFramework.setViewOverlayDisplay(displayOverlay);
expect(UiFramework.viewOverlayDisplay).to.eql(displayOverlay);
// test workflow that doesn't change the item
const currentDisplay = UiFramework.viewOverlayDisplay;
UiFramework.setViewOverlayDisplay(displayOverlay);
expect(UiFramework.viewOverlayDisplay).to.eql(currentDisplay);

TestUtils.terminateUiFramework();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ describe("StandardMessageBox", () => {
const reactNode = <StandardMessageBox
opened={true}
title="My Title"
iconType={MessageBoxIconType.NoSymbol}
iconType={MessageBoxIconType.Success}
messageBoxType={MessageBoxType.Ok}
onResult={spyOnEscape}
/>;
Expand Down
5 changes: 4 additions & 1 deletion ui/appui-react/src/test/messages/MessageManager.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,10 @@ describe("MessageManager", () => {
expect(MessageManager.getSeverity(details)).to.eq(MessageSeverity.Fatal);

details = new NotifyMessageDetails(OutputMessagePriority.None, "A brief message.");
expect(MessageManager.getSeverity(details)).to.eq(MessageSeverity.None);
expect(MessageManager.getSeverity(details)).to.eq(MessageSeverity.Success);

details = new NotifyMessageDetails(OutputMessagePriority.Success, "A brief message.");
expect(MessageManager.getSeverity(details)).to.eq(MessageSeverity.Success);
});

it("non-duplicate message should be added to Message Center", () => {
Expand Down
3 changes: 3 additions & 0 deletions ui/core-react/src/core-react/messagebox/MessageBox.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@ export class MessageContainer extends React.PureComponent<MessageContainerProps>

switch (severity) {
case MessageSeverity.None:
iconClassName = " ";
break;
case MessageSeverity.Success:
iconClassName = hollow ? "icon-status-success-hollow" : "icon-status-success" + " core-message-box-success";
break;
case MessageSeverity.Information:
Expand Down
7 changes: 7 additions & 0 deletions ui/core-react/src/test/messagebox/MessageBox.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -57,8 +57,14 @@ describe("MessageBox", () => {
it("MessageSeverity.None", () => {
const wrapper = mount(<MessageBox opened={true} severity={MessageSeverity.None} buttonCluster={buttonCluster} />);
const icon = wrapper.find("div.core-message-box-success");
expect(icon.length).to.eq(0);
});
it("MessageSeverity.Success", () => {
const wrapper = mount(<MessageBox opened={true} severity={MessageSeverity.Success} buttonCluster={buttonCluster} />);
const icon = wrapper.find("div.core-message-box-success");
expect(icon.length).to.eq(1);
});

});

describe("MessageContainer.getIconClassName with hollow param", () => {
Expand All @@ -69,6 +75,7 @@ describe("MessageBox", () => {
expect(MessageContainer.getIconClassName(MessageSeverity.Warning, true).length).to.not.eq(0);
expect(MessageContainer.getIconClassName(MessageSeverity.Error, true).length).to.not.eq(0);
expect(MessageContainer.getIconClassName(MessageSeverity.Fatal, true).length).to.not.eq(0);
expect(MessageContainer.getIconClassName(MessageSeverity.Success, true).length).to.not.eq(0);
});
});

Expand Down