-
Notifications
You must be signed in to change notification settings - Fork 415
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
Remove HomePageViewController code #3454
base: main
Are you sure you want to change the base?
Conversation
9eb444c
to
0653f8e
Compare
Hi @dus7 I have no context about these changes and the task is empty, could you provide some info and testing steps? |
@@ -169,3 +169,12 @@ struct HomeMessageButtonViewModel { | |||
let action: () async -> Void | |||
|
|||
} | |||
|
|||
private extension RemoteAction { |
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.
Extracted out of HomeMessageViewSectionRenderer.swift
which was deleted.
@@ -83,13 +84,6 @@ class FavoritesHomeViewSectionRenderer: NSObject, HomeViewSectionRenderer { | |||
return Constants.defaultHeaderHeight | |||
} | |||
|
|||
func install(into controller: HomeViewController) { |
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 one is no longer necessary since there is no HomeViewController
.
@@ -43,7 +43,8 @@ class FavoritesHomeViewSectionRenderer: NSObject, HomeViewSectionRenderer { | |||
static let defaultHeaderHeight: CGFloat = 20 | |||
static let horizontalMargin: CGFloat = 2 | |||
static let largeModeMargin: CGFloat = 24 | |||
|
|||
static let sideInsets: CGFloat = 25 |
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.
Extracted from HomeViewSectionRenderers.swift
file which was deleted.
@@ -27,21 +27,6 @@ import os.log | |||
|
|||
final class HomePageConfiguration: HomePageMessagesConfiguration { | |||
|
|||
enum Component: Equatable { |
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.
Removed since it was used only by HomeViewController
which is also removed.
Hi, I added a few inline comments and an initial PR link in the description. This PR does not cause any UI and/or logic changes. All the modifications were done in previous one (#3453). This is just removing unused code. |
Task/Issue URL: https://app.asana.com/0/72649045549333/1208547223289954/f
Tech Design URL:
CC: @brindy
Description:
This is a follow up after #3453.
Removes unused code related to HomePageViewController. Home renderers abstraction was also removed, though the implementation itself is used in both Bookmarks controller and to show Favorites while editing URL on non-home tab.
Steps to test this PR:
Definition of Done (Internal Only):
Copy Testing:
’
rather than'
Orientation Testing:
Device Testing:
OS Testing:
Theme Testing:
Internal references:
Software Engineering Expectations
Technical Design Template