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

[MBL-983] Already Reported Project Label #1857

Merged
merged 22 commits into from
Sep 28, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
22 commits
Select commit Hold shift + click to select a range
e49b140
create ReportProjectCell to display swiftui view
scottkicks Sep 22, 2023
bc5838c
add new cell below overviewSubpages and pass in project's flagging prop
scottkicks Sep 22, 2023
a0b0f3a
rename label view
scottkicks Sep 23, 2023
ca22858
fix precondition failure: cyclic graph
scottkicks Sep 23, 2023
5da15cc
layout tableview if needed
scottkicks Sep 23, 2023
65171d1
conditionally render labels using `flagged` property
scottkicks Sep 23, 2023
b4400cd
cleanup
scottkicks Sep 23, 2023
5dabfa7
pass project flagged property along with projectURL to viewcontroller…
scottkicks Sep 23, 2023
a692182
remove temp fix for cyclic graph crash
scottkicks Sep 23, 2023
c6e7492
update tests
scottkicks Sep 23, 2023
57fd4cd
updateProjectPageViewControllerDataSourceTests tests
scottkicks Sep 23, 2023
8b3eacf
fix pamphletsubpage cell divider
scottkicks Sep 25, 2023
cdc65d4
fix view styling on iPad
scottkicks Sep 25, 2023
301cfb7
Merge branch 'mbl-983-already-reported-label' of https://github.com/k…
scottkicks Sep 25, 2023
00c21f4
formatting
scottkicks Sep 25, 2023
ce8e695
support more than one hyperlink in a string
scottkicks Sep 25, 2023
46528de
undo change in deprecated file
scottkicks Sep 27, 2023
3319b02
update hyperlink urls
scottkicks Sep 27, 2023
18549e8
set accessibility traits and cell's traitCollection
scottkicks Sep 27, 2023
1bbe0ec
Merge branch 'main' into mbl-983-already-reported-label
scottkicks Sep 27, 2023
62e8c77
Merge branch 'mbl-983-already-reported-label' of https://github.com/k…
scottkicks Sep 28, 2023
b38f921
pop views on successful submissionupdate contraint margins
scottkicks Sep 28, 2023
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,7 @@
self.tableView.registerCellClass(ImageViewElementCell.self)
self.tableView.registerCellClass(AudioVideoViewElementCell.self)
self.tableView.registerCellClass(ExternalSourceViewElementCell.self)
self.tableView.registerCellClass(ReportProjectCell.self)
self.tableView.register(nib: .ProjectPamphletMainCell)
self.tableView.register(nib: .ProjectPamphletSubpageCell)
self.tableView.registerCellClass(ProjectRisksCell.self)
Expand Down Expand Up @@ -382,8 +383,9 @@

self.viewModel.outputs.goToReportProject
.observeForControllerAction()
.observeValues { [weak self] in
self?.goToReportProject(projectUrl: $0)
.observeValues { [weak self] flagged, projectUrl in
guard !flagged else { return }
self?.goToReportProject(projectUrl: projectUrl)

Check warning on line 388 in Kickstarter-iOS/Features/ProjectPage/Controller/ProjectPageViewController.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Controller/ProjectPageViewController.swift#L388

Added line #L388 was not covered by tests
}

self.viewModel.outputs.goToUpdates
Expand Down Expand Up @@ -829,9 +831,9 @@
self.viewModel.inputs.tappedComments()
} else if self.dataSource.indexPathIsUpdatesSubpage(indexPath) {
self.viewModel.inputs.tappedUpdates()
} else if self.dataSource.indexPathIsReportProject(indexPath) {
self.viewModel.inputs.tappedReportProject()
}
case ProjectPageViewControllerDataSource.Section.overviewReportProject.rawValue:
self.viewModel.inputs.tappedReportProject()
case ProjectPageViewControllerDataSource.Section.faqsAskAQuestion.rawValue:
self.viewModel.inputs.askAQuestionCellTapped()
case ProjectPageViewControllerDataSource.Section.faqs.rawValue:
Expand Down Expand Up @@ -863,7 +865,9 @@

/// If we are displaying the `ProjectPamphletSubpageCell` we do not want to show the cells separator.
self.tableView.separatorStyle = indexPath.section == ProjectPageViewControllerDataSource.Section
.overviewSubpages.rawValue ? .none : .singleLine
.overviewReportProject.rawValue ? .none : .singleLine

self.tableView.layoutIfNeeded()
}

public func tableView(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ internal final class ProjectPageViewControllerDataSource: ValueCellDataSource {
case overviewCreatorHeader
case overview
case overviewSubpages
case overviewReportProject
case campaignHeader
case campaign
case faqsHeader
Expand Down Expand Up @@ -104,15 +105,20 @@ internal final class ProjectPageViewControllerDataSource: ValueCellDataSource {

let values: [ProjectPamphletSubpage] = [
.comments(project.stats.commentsCount as Int?, .first),
.updates(project.stats.updatesCount as Int?, .middle),
.reportProject(.last)
.updates(project.stats.updatesCount as Int?, .middle)
]

self.set(
values: values,
cellClass: ProjectPamphletSubpageCell.self,
inSection: Section.overviewSubpages.rawValue
)

self.set(
values: [project.flagging ?? false],
cellClass: ReportProjectCell.self,
inSection: Section.overviewReportProject.rawValue
)
case .campaign:
self.set(
values: [HeaderValue.campaign.description],
Expand Down Expand Up @@ -339,6 +345,8 @@ internal final class ProjectPageViewControllerDataSource: ValueCellDataSource {
cell.configureWith(value: value)
case let (cell as ExternalSourceViewElementCell, value as ExternalSourceViewElement):
cell.configureWith(value: value)
case let (cell as ReportProjectCell, value as Bool):
cell.configureWith(value: value)
default:
assertionFailure("Unrecognized combo: \(cell), \(value)")
}
Expand Down Expand Up @@ -449,10 +457,6 @@ internal final class ProjectPageViewControllerDataSource: ValueCellDataSource {
return (self[indexPath] as? ProjectPamphletSubpage)?.isUpdates == true
}

internal func indexPathIsReportProject(_ indexPath: IndexPath) -> Bool {
return (self[indexPath] as? ProjectPamphletSubpage)?.isReportProject == true
}

internal func isExpandedValuesForFAQsSection() -> [Bool]? {
guard let values = self[section: Section.faqs.rawValue] as? [(ProjectFAQ, Bool)] else { return nil }
return values.map { _, isExpanded in isExpanded }
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,8 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
.rawValue
private let overviewSection = ProjectPageViewControllerDataSource.Section.overview.rawValue
private let overviewSubpagesSection = ProjectPageViewControllerDataSource.Section.overviewSubpages.rawValue
private let overviewReportProject = ProjectPageViewControllerDataSource.Section.overviewReportProject
.rawValue
private let faqsHeaderSection = ProjectPageViewControllerDataSource.Section.faqsHeader.rawValue
private let faqsEmptySection = ProjectPageViewControllerDataSource.Section.faqsEmpty.rawValue
private let faqsSection = ProjectPageViewControllerDataSource.Section.faqs.rawValue
Expand Down Expand Up @@ -144,7 +146,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: [false, false, false, false]
)
XCTAssertEqual(9, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(10, self.dataSource.numberOfSections(in: self.tableView))

// faqsHeader
XCTAssertEqual(
Expand Down Expand Up @@ -200,7 +202,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: [false, false, false, false]
)
XCTAssertEqual(8, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(9, self.dataSource.numberOfSections(in: self.tableView))

// faqsHeader
XCTAssertEqual(
Expand Down Expand Up @@ -245,7 +247,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
project: project,
refTag: nil
)
XCTAssertEqual(9, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(10, self.dataSource.numberOfSections(in: self.tableView))

// faqsHeader
XCTAssertEqual(
Expand Down Expand Up @@ -300,7 +302,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
project: project,
refTag: nil
)
XCTAssertEqual(7, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(8, self.dataSource.numberOfSections(in: self.tableView))

// faqsHeader
XCTAssertEqual(
Expand Down Expand Up @@ -343,7 +345,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: nil
)
XCTAssertEqual(12, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(13, self.dataSource.numberOfSections(in: self.tableView))

// risksHeader
XCTAssertEqual(
Expand Down Expand Up @@ -396,7 +398,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: nil
)
XCTAssertEqual(17, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(18, self.dataSource.numberOfSections(in: self.tableView))

XCTAssertEqual(
1,
Expand Down Expand Up @@ -477,7 +479,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: nil
)
XCTAssertEqual(17, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(18, self.dataSource.numberOfSections(in: self.tableView))

XCTAssertEqual(
1,
Expand Down Expand Up @@ -551,7 +553,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: nil
)
XCTAssertEqual(20, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(21, self.dataSource.numberOfSections(in: self.tableView))

// environmentCommitmentsHeader
XCTAssertEqual(
Expand Down Expand Up @@ -607,7 +609,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: nil
)
XCTAssertEqual(20, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(21, self.dataSource.numberOfSections(in: self.tableView))

// environmentCommitmentsHeader
XCTAssertEqual(
Expand Down Expand Up @@ -659,7 +661,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: nil
)
XCTAssertEqual(5, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(6, self.dataSource.numberOfSections(in: self.tableView))

// campaign header section
XCTAssertEqual(
Expand Down Expand Up @@ -715,7 +717,7 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
refTag: nil,
isExpandedStates: nil
)
XCTAssertEqual(3, self.dataSource.numberOfSections(in: self.tableView))
XCTAssertEqual(4, self.dataSource.numberOfSections(in: self.tableView))

// overviewCreatorHeader
XCTAssertEqual(
Expand All @@ -731,10 +733,15 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {

// overviewSubpages
XCTAssertEqual(
3,
2,
self.dataSource.tableView(self.tableView, numberOfRowsInSection: self.overviewSubpagesSection)
)

XCTAssertEqual(
1,
self.dataSource.tableView(self.tableView, numberOfRowsInSection: self.overviewReportProject)
)

XCTAssertEqual(
"ProjectPamphletMainCell",
self.dataSource.reusableId(item: 0, section: self.overviewSection)
Expand All @@ -743,6 +750,11 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
"ProjectPamphletSubpageCell",
self.dataSource.reusableId(item: 0, section: self.overviewSubpagesSection)
)

XCTAssertEqual(
"ReportProjectCell",
self.dataSource.reusableId(item: 0, section: self.overviewReportProject)
)
}
}

Expand Down Expand Up @@ -818,31 +830,6 @@ final class ProjectPageViewControllerDataSourceTests: XCTestCase {
)
}

func testIndexPathIsReportProjectSubpage() {
let project = Project.template
|> \.displayPrelaunch .~ false
|> \.extendedProjectProperties .~ ExtendedProjectProperties(
environmentalCommitments: [],
faqs: [],
aiDisclosure: nil,
risks: "",
story: self.storyViewableElements,
minimumPledgeAmount: 1
)

self.dataSource.load(
navigationSection: .overview,
project: project,
refTag: nil
)

XCTAssertEqual(
self.dataSource
.indexPathIsReportProject(IndexPath(row: 2, section: self.overviewSubpagesSection)),
true
)
}

func testUpdatingCampaign_WithImageViewElementImage_Success() {
let project = Project.template
|> \.extendedProjectProperties .~ ExtendedProjectProperties(
Expand Down
Copy link
Contributor

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?

Copy link
Contributor Author

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

Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import Library
import Prelude
import SwiftUI
import UIKit

internal final class ReportProjectCell: UITableViewCell, ValueCell {
internal func configureWith(value projectFlagged: Bool) {
self.setupTableViewCellStyle(projectFlagged: projectFlagged)
self.setupReportProjectLabelView(projectFlagged: projectFlagged)
}

internal override func layoutSubviews() {
super.layoutSubviews()
}

internal override func bindStyles() {
super.bindStyles()

self.separatorInset = UIEdgeInsets(top: 0, left: 0, bottom: 0, right: .greatestFiniteMagnitude)
self.setNeedsLayout()
}

// MARK: - Private Methods

private func setupTableViewCellStyle(projectFlagged: Bool) {
let accessibilityTraits = projectFlagged
? UIAccessibilityTraits.staticText
: UIAccessibilityTraits.button

_ = self
|> baseTableViewCellStyle()
|> ReportProjectCell.lens.accessibilityTraits .~ accessibilityTraits
Comment on lines +26 to +32
Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks for adding this! Unfortunately, I don't think it does anything currently; we'd need to also set isAccessibilityElement = true and the accessibility label. I don't see an easy way to set the label based on the swiftui child view. So I'd suggest either leaving this alone for now or adding it just for the button state. Ex:

if projectFlagged  {
  self.accessibilityTraits = super.accessibilityTraits
  self.isAccessibilityElement = false
} else {
  self.accessibilityTraits.insert(.button)
  self.isAccessibilityElement = true
  self.accessibilityLabel = Strings.Report_this_project_to()
}

Copy link
Contributor Author

Choose a reason for hiding this comment

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

oh, good callout! I'll look into this in the next PR

}

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)

let leftRightInset = self.traitCollection.isRegularRegular ? Styles.grid(16) : Styles.gridHalf(5)

NSLayoutConstraint.activate([
hostingController.view.topAnchor
.constraint(equalTo: self.contentView.topAnchor, constant: Styles.gridHalf(5)),
hostingController.view.bottomAnchor
.constraint(equalTo: self.contentView.bottomAnchor, constant: -Styles.gridHalf(5)),
hostingController.view.leadingAnchor
.constraint(equalTo: self.contentView.leadingAnchor, constant: leftRightInset),
hostingController.view.trailingAnchor
.constraint(equalTo: self.contentView.trailingAnchor, constant: -leftRightInset)
])
}
}
}
}
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

@SwiftUI.Environment(\.horizontalSizeClass) private var horizontalSizeClass

var body: some View {
if flagged {
AlreadyReportedView()

Check warning on line 12 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L12

Added line #L12 was not covered by tests
} else {
HStack {
Text(Strings.Report_this_project_to())
.font(Font(UIFont.ksr_body(size: 14)))
.foregroundColor(Color(.ksr_support_700))

Check warning on line 17 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L15-L17

Added lines #L15 - L17 were not covered by tests

Spacer()

Check warning on line 19 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L19

Added line #L19 was not covered by tests

Image("chevron-right")
.resizable()
.scaledToFit()
.frame(width: 10, height: 10)

Check warning on line 24 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L21-L24

Added lines #L21 - L24 were not covered by tests
}
.padding(self.horizontalSizeClass == .regular ? 0 : 10)

Check warning on line 26 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L26

Added line #L26 was not covered by tests
}
}

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))

Check warning on line 37 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L33-L37

Added lines #L33 - L37 were not covered by tests

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)?

Check warning on line 42 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L39-L42

Added lines #L39 - L42 were not covered by tests
.absoluteString ?? "",
community_guidelines: HelpType.community
.url(withBaseUrl: AppEnvironment.current.apiService.serverConfig.webBaseUrl)?

Check warning on line 45 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L44-L45

Added lines #L44 - L45 were not covered by tests
.absoluteString ?? ""
),
with: [
ReportProjectHyperLinkType.ourRules.stringLiteral(),
ReportProjectHyperLinkType.communityGuidelines.stringLiteral()
]
)
.font(Font(UIFont.ksr_caption1()))

Check warning on line 53 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L47-L53

Added lines #L47 - L53 were not covered by tests
}
.padding()
.background(Color(.ksr_support_100))
.cornerRadius(15)

Check warning on line 57 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L55-L57

Added lines #L55 - L57 were not covered by tests
}
}
}

@available(iOS 15.0, *)
struct ReportProjectView_Previews: PreviewProvider {
static var previews: some View {
ReportProjectLabelView(flagged: false)

Check warning on line 65 in Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift

View check run for this annotation

Codecov / codecov/patch

Kickstarter-iOS/Features/ProjectPage/Views/ReportProjectLabelView.swift#L65

Added line #L65 was not covered by tests
}
}