-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
[MBL-983] Already Reported Project Label #1857
Changes from 16 commits
e49b140
bc5838c
a0b0f3a
ca22858
5da15cc
65171d1
b4400cd
5dabfa7
a692182
c6e7492
57fd4cd
8b3eacf
cdc65d4
301cfb7
00c21f4
ce8e695
46528de
3319b02
18549e8
1bbe0ec
62e8c77
b38f921
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 |
---|---|---|
@@ -0,0 +1,48 @@ | ||
import Library | ||
import SwiftUI | ||
import UIKit | ||
|
||
internal final class ReportProjectCell: UITableViewCell, ValueCell { | ||
let isIpad = AppEnvironment.current.device.userInterfaceIdiom == .pad | ||
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. This should look at the horizontal size class instead. This is important to make layouts look good on iPad split screen, for example. |
||
|
||
internal func configureWith(value projectFlagged: Bool) { | ||
self.setupReportProjectLabelView(projectFlagged: projectFlagged) | ||
|
||
self.selectionStyle = .none | ||
self.separatorInset = UIEdgeInsets(top: 0, left: 0, bottom: 0, right: .greatestFiniteMagnitude) | ||
self.setNeedsLayout() | ||
} | ||
|
||
internal override func layoutSubviews() { | ||
super.layoutSubviews() | ||
} | ||
|
||
private func setupReportProjectLabelView(projectFlagged: Bool) { | ||
if #available(iOS 15.0, *) { | ||
DispatchQueue.main.async { | ||
let hostingController = UIHostingController(rootView: ReportProjectLabelView(flagged: projectFlagged)) | ||
|
||
hostingController.view.translatesAutoresizingMaskIntoConstraints = false | ||
hostingController.view.backgroundColor = .clear | ||
|
||
self.contentView.addSubview(hostingController.view) | ||
|
||
NSLayoutConstraint.activate([ | ||
hostingController.view.topAnchor | ||
.constraint(equalTo: self.contentView.topAnchor, constant: self.isIpad ? Styles.grid(5) : 12), | ||
hostingController.view.bottomAnchor.constraint(equalTo: self.contentView.bottomAnchor), | ||
hostingController.view.leadingAnchor | ||
.constraint( | ||
equalTo: self.contentView.leadingAnchor, | ||
constant: self.isIpad ? Styles.grid(16) : 12 | ||
), | ||
hostingController.view.trailingAnchor | ||
.constraint( | ||
equalTo: self.contentView.trailingAnchor, | ||
constant: self.isIpad ? -Styles.grid(16) : -12 | ||
) | ||
]) | ||
} | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,67 @@ | ||
import Library | ||
import SwiftUI | ||
|
||
@available(iOS 15.0, *) | ||
struct ReportProjectLabelView: View { | ||
let flagged: Bool | ||
|
||
let isIpad = AppEnvironment.current.device.userInterfaceIdiom == .pad | ||
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. Same here; look at the size class instead of the device type. |
||
|
||
var body: some View { | ||
if flagged { | ||
AlreadyReportedView() | ||
} else { | ||
HStack { | ||
Text(Strings.Report_this_project_to()) | ||
.font(Font(UIFont.ksr_footnote())) | ||
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. I'd suggest using |
||
.foregroundColor(Color(.ksr_support_700)) | ||
|
||
Spacer() | ||
|
||
Image("chevron-right") | ||
.resizable() | ||
.scaledToFit() | ||
.frame(width: 10, height: 10) | ||
} | ||
.padding(isIpad ? 0 : 10) | ||
} | ||
} | ||
|
||
private struct AlreadyReportedView: View { | ||
var body: some View { | ||
HStack(alignment: .top, spacing: 10) { | ||
Image("info") | ||
.resizable() | ||
.scaledToFit() | ||
.frame(width: 20, height: 20) | ||
.foregroundColor(Color(.ksr_support_500)) | ||
|
||
Text( | ||
html: Strings.It_looks( | ||
our_rules: HelpType.prohibitedItems | ||
scottkicks marked this conversation as resolved.
Show resolved
Hide resolved
|
||
.url(withBaseUrl: AppEnvironment.current.apiService.serverConfig.webBaseUrl)? | ||
.absoluteString ?? "", | ||
community_guidelines: HelpType.prohibitedItems | ||
.url(withBaseUrl: AppEnvironment.current.apiService.serverConfig.webBaseUrl)? | ||
.absoluteString ?? "" | ||
), | ||
with: [ | ||
ReportProjectHyperLinkType.ourRules.stringLiteral(), | ||
ReportProjectHyperLinkType.communityGuidelines.stringLiteral() | ||
] | ||
) | ||
.font(Font(UIFont.ksr_caption1())) | ||
} | ||
.padding() | ||
.background(Color(.ksr_support_100)) | ||
.cornerRadius(15) | ||
} | ||
} | ||
} | ||
|
||
@available(iOS 15.0, *) | ||
struct ReportProjectView_Previews: PreviewProvider { | ||
static var previews: some View { | ||
ReportProjectLabelView(flagged: false) | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,7 @@ internal final class ProjectPamphletContentDataSource: ValueCellDataSource { | |
|
||
let values = [ | ||
ProjectPamphletSubpage.comments(project.stats.commentsCount as Int?, .first), | ||
ProjectPamphletSubpage.updates(project.stats.updatesCount as Int?, .last) | ||
ProjectPamphletSubpage.updates(project.stats.updatesCount as Int?, .middle) | ||
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. nit: This file is marked as deprecated - I'm not sure we need this change (or the file?) 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. Ah yea, good catch. was just making sure all uses of these cells had the right positioning. I'll undo this change. Looking forward to removing this file completely. |
||
] | ||
|
||
self.set( | ||
|
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.
I think this cell is still clickable, even when it's already flagged. I'm not 100% sure, since I just overwrote the
flagged
bool locally to test this (instead of contacting team A to get myself added to the flagged list). Will you double check and make the cell non-interactive if needed?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.
double checked and the "already reported" label is never tappable. Only when flagged is false do we show the tappable "report this project" label