-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Show a toast after successfully submitting a comment. #25945
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
base: trunk
Are you sure you want to change the base?
Changes from all commits
42e8998
b3cc83b
3dd5d5a
a9072f3
f2f29fc
7ea9a7b
2579231
0196fe7
cddc957
3c9a75c
7b7d7c3
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,6 @@ | ||
| import UIKit | ||
| import WordPressShared | ||
| import DesignSystem | ||
|
|
||
| public enum NoticeAnimationStyle { | ||
| case moveIn | ||
|
|
@@ -73,9 +74,46 @@ public struct NormalNoticeStyle: NoticeStyle { | |
|
|
||
| public struct InAppUpdateNoticeStyle: NoticeStyle { | ||
| public let attributedMessage: NSAttributedString? | ||
|
|
||
| init(attributedMessage: NSAttributedString? = nil) { | ||
| self.attributedMessage = attributedMessage | ||
| public let isDismissable: Bool | ||
|
|
||
| /// - Parameters: | ||
| /// - icon: An optional SF Symbol rendered inline before `title`, e.g. a checkmark seal to indicate success. | ||
| /// - iconColor: The tint color applied to `icon`. | ||
| /// - title: When provided (with or without `icon`), builds `attributedMessage` from it and makes the | ||
| /// Notice auto-dismiss after a few seconds. When `nil`, the Notice falls back to its own `title`/`message` | ||
| /// and stays on screen until the user dismisses it, matching the original in-app-update banner behavior. | ||
| init(icon: UIImage? = nil, iconColor: UIColor = .invertedLabel, title: String? = nil) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason for adding this new initializer? Does the default Notice work?
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
And before we were only accepting Before it didn't have support of the
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just on clarification, before this Notice was not dismissible, it stays on the screen until user removed it, its value was set to false, now it can get auto-dismiss after few seconds. I can revert this just one line change, if there is no need. Please clarify on this one.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reusing the existing style should be fine, like Notice(title:...). The notice auto dismisses after a few seconds, and users can tap it to dismiss it, which should all be the default UX.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup, that is how it's working, either it will dismiss after sometime, or user can dismiss it before that time. |
||
| guard let title else { | ||
| self.attributedMessage = nil | ||
| self.isDismissable = false | ||
| return | ||
| } | ||
|
|
||
| self.isDismissable = true | ||
|
|
||
| let font = UIFont.boldSystemFont(ofSize: 14.0) | ||
| let message = NSMutableAttributedString() | ||
|
|
||
| if let icon = icon?.withTintColor(iconColor, renderingMode: .alwaysOriginal) { | ||
| let attachment = NSTextAttachment(image: icon) | ||
| attachment.accessibilityLabel = "" // Decorative; the title text conveys the meaning. | ||
| let iconHeight = font.lineHeight | ||
| let ratio = icon.size.width / icon.size.height | ||
| attachment.bounds = CGRect( | ||
| x: 0, | ||
| y: (font.capHeight - iconHeight) / 2, | ||
| width: iconHeight * ratio, | ||
| height: iconHeight | ||
| ) | ||
| message.append(NSAttributedString(attachment: attachment)) | ||
| message.append(NSAttributedString(string: " ")) | ||
| } | ||
|
|
||
| message.append( | ||
| NSAttributedString(string: title, attributes: [.font: font, .foregroundColor: UIColor.invertedLabel]) | ||
| ) | ||
|
|
||
| self.attributedMessage = message | ||
| } | ||
|
|
||
| // Return new UIFont instance everytime in order to be responsive to accessibility font size changes | ||
|
|
@@ -85,7 +123,6 @@ public struct InAppUpdateNoticeStyle: NoticeStyle { | |
|
|
||
| public let directionalLayoutMargins = NSDirectionalEdgeInsets(top: 13.0, leading: 16.0, bottom: 13.0, trailing: 16.0) | ||
|
|
||
| public var isDismissable = false | ||
| public let showNextArrow = false | ||
|
|
||
| public let animationStyle = NoticeAnimationStyle.moveIn | ||
|
|
||
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.
This notice does not apply to all sites, right? Some site you can just publish without being reviewed. Like, if you are member of another site, your comments on that site should go straight to approved?
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.
ok will update according to that 🤔
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.
Updated Code, now toast will show only when comment is unapproved.
The core problem: the composer had no way to know whether a comment was actually held for moderation. I traced the real status through the whole call chain and made it flow back to the UI.