Skip to content

Commit

Permalink
Improve the error message alert panel (#84)
Browse files Browse the repository at this point in the history
  • Loading branch information
sindresorhus committed Jun 22, 2019
1 parent 7929e86 commit cfcaa36
Show file tree
Hide file tree
Showing 2 changed files with 68 additions and 7 deletions.
11 changes: 5 additions & 6 deletions Gifski/MainWindowController.swift
Expand Up @@ -141,11 +141,10 @@ final class MainWindowController: NSWindowController {
}

if asset.hasAudio && !asset.hasVideo {
NSAlert.showModalAndReportToCrashlytics(
NSAlert.showModal(
for: window,
message: "Audio files are not supported.",
informativeText: "Gifski converts video files but the provided file is audio-only. Please provide a file that contains video.",
debugInfo: asset.debugInfo
informativeText: "Gifski converts video files but the provided file is audio-only. Please provide a file that contains video."
)

return
Expand All @@ -156,7 +155,7 @@ final class MainWindowController: NSWindowController {
NSAlert.showModalAndReportToCrashlytics(
for: window,
message: "The video file is not supported.",
informativeText: "Please open an issue on https://github.com/sindresorhus/Gifski or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:\n\(asset.debugInfo)",
informativeText: "Please open an issue on https://github.com/sindresorhus/Gifski or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:",
debugInfo: asset.debugInfo
)

Expand All @@ -167,7 +166,7 @@ final class MainWindowController: NSWindowController {
NSAlert.showModalAndReportToCrashlytics(
for: window,
message: "The video metadata is not readable.",
informativeText: "Please open an issue on https://github.com/sindresorhus/Gifski or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:\n\(asset.debugInfo)",
informativeText: "Please open an issue on https://github.com/sindresorhus/Gifski or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:",
debugInfo: asset.debugInfo
)

Expand All @@ -182,7 +181,7 @@ final class MainWindowController: NSWindowController {
NSAlert.showModalAndReportToCrashlytics(
for: window,
message: "The video dimensions must be at least 10×10.",
informativeText: "The dimensions of your video are \(asset.dimensions?.formatted ?? "0×0").\n\nIf you think this error is a mistake, please open an issue on https://github.com/sindresorhus/Gifski or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:\n\(asset.debugInfo)",
informativeText: "The dimensions of your video are \(asset.dimensions?.formatted ?? "0×0").\n\nIf you think this error is a mistake, please open an issue on https://github.com/sindresorhus/Gifski or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:",
debugInfo: asset.debugInfo
)

Expand Down
64 changes: 63 additions & 1 deletion Gifski/util.swift
Expand Up @@ -200,7 +200,36 @@ extension NSWindowController: NSWindowDelegate {
}


extension NSView {
private final class AddedToSuperviewObserverView: NSView {
var onAdded: (() -> Void)?

override var acceptsFirstResponder: Bool {
return false
}

convenience init() {
self.init(frame: .zero)
}

override func viewDidMoveToWindow() {
guard window != nil else {
return
}

onAdded?()
removeFromSuperview()
}
}

// TODO: Please show me a better way to achieve this than using an invisible view 🙏
/// Enables you do add contraints and do other initialization without having to subclass the view.
func onAddedToSuperview(_ closure: @escaping () -> Void) {
let view = AddedToSuperviewObserverView()
view.onAdded = closure
addSubview(view)
}
}

extension NSAlert {
/// Show an alert as a window-modal sheet, or as an app-modal (window-indepedendent) alert if the window is `nil` or not given.
Expand All @@ -209,18 +238,21 @@ extension NSAlert {
for window: NSWindow? = nil,
message: String,
informativeText: String? = nil,
detailText: String? = nil,
style: NSAlert.Style = .warning
) -> NSApplication.ModalResponse {
return NSAlert(
message: message,
informativeText: informativeText,
detailText: detailText,
style: style
).runModal(for: window)
}

convenience init(
message: String,
informativeText: String? = nil,
detailText: String? = nil,
style: NSAlert.Style = .warning
) {
self.init()
Expand All @@ -230,6 +262,35 @@ extension NSAlert {
if let informativeText = informativeText {
self.informativeText = informativeText
}

if let detailText = detailText {
if #available(macOS 10.14, *) {
let scrollView = NSTextView.scrollableTextView()

// We're setting the frame manually here as it's impossible to use auto-layout,
// since it has nothing to constrain to. This will eventually be rewritten in SwiftUI anyway.
scrollView.frame = CGRect(width: 300, height: 120)

scrollView.onAddedToSuperview {
if let messageTextField = (scrollView.superview?.superview?.subviews.first { $0 is NSTextField }) {
scrollView.frame.width = messageTextField.frame.width
} else {
assertionFailure("Couldn't detect the message textfield view of the NSAlert panel")
}
}

let textView = scrollView.documentView as! NSTextView
textView.drawsBackground = false
textView.isEditable = false
textView.font = NSFont.systemFont(ofSize: NSFont.systemFontSize(for: .small))
textView.textColor = .secondaryLabelColor
textView.string = detailText

self.accessoryView = scrollView
} else {
self.informativeText += "\n\(detailText)"
}
}
}

/// Runs the alert as a window-modal sheet, or as an app-modal (window-indepedendent) alert if the window is `nil` or not given.
Expand Down Expand Up @@ -684,7 +745,7 @@ extension AVFormat: CustomStringConvertible {

extension AVFormat: CustomDebugStringConvertible {
var debugDescription: String {
return "\(description) (\(fourCC))"
return "\(description) (\(fourCC.trimmingCharacters(in: .whitespaces)))"
}
}

Expand Down Expand Up @@ -1971,6 +2032,7 @@ extension Dictionary {
for: window,
message: message,
informativeText: informativeText,
detailText: debugInfo,
style: style
)
}
Expand Down

0 comments on commit cfcaa36

Please sign in to comment.