Add FXIOS-12363 ⁃ [Menu Redesign] Update horizontal squares options to the latest design #27115
Conversation
…s to the latest design
Client.app: Coverage: 35.41
Generated by 🚫 Danger Swift against 9c5dcd5 |
BrowserKit/Sources/MenuKit/MenuRedesign/MenuCollectionView.swift
Outdated
Show resolved
Hide resolved
| if tabInfo.isHomepage { | ||
| menuSections.append(getNewTabSection(with: uuid, tabInfo: tabInfo)) | ||
| menuSections.append(getHorizontalTabsSection(with: uuid, tabInfo: tabInfo)) | ||
| } else { | ||
| menuSections.append(getSiteSection(with: uuid, tabInfo: tabInfo, isExpanded: isExpanded)) | ||
| } |
There was a problem hiding this comment.
We could merge else with inner if like: else if tabInfo.isHomepage { for readability
| cell.configureCellWith(model: section.options[indexPath.row]) | ||
| if let theme { cell.applyTheme(theme: theme) } | ||
| return cell | ||
| } | ||
|
|
||
| func collectionView(_ collectionView: UICollectionView, didSelectItemAt indexPath: IndexPath) { | ||
| collectionView.deselectItem(at: indexPath, animated: false) | ||
| guard let section = menuData.first(where: { $0.isTopTabsSection }), | ||
| guard let section = menuData.first(where: { $0.isHorizontalTabsSection }), |
There was a problem hiding this comment.
this check is done multiple times can be a computed var to avoid repeating the same code like
var horizontalTabsSection: MenuSection? {
return menuData.first(where: { $0.isHorizontalTabsSection })
}
| @@ -77,11 +84,29 @@ final class MenuRedesignTableView: UIView, | |||
| return section == 0 ? UX.topPadding : UX.distanceBetweenSections | |||
| } | |||
|
|
|||
| func tableView(_ tableView: UITableView, heightForRowAt indexPath: IndexPath) -> CGFloat { | |||
| if menuData[indexPath.section].isHorizontalTabsSection { | |||
There was a problem hiding this comment.
We should avoid calculating the height manually, I understand it is a cell containing a collection view but let's explore other solutions.
As last resort if we don't find a solution I think this logic should be in MenuCollectionView or MenuSquareCell instead of here
| @@ -159,11 +174,11 @@ final class MenuRedesignTableView: UIView, | |||
| } | |||
|
|
|||
| func reloadTableView(with data: [MenuSection]) { | |||
| // We ignore first section because it is handled in MenuCollectionView | |||
| if let firstSection = data.first, firstSection.isTopTabsSection { | |||
| // We ignore first section because it is handled in MenuSquaresViewContentCell | |||
There was a problem hiding this comment.
nit: I think the comment should be updated. We don't ignore the section just handled independently
| } | ||
|
|
||
| func reloadData(with data: [MenuSection]) { | ||
| menuData = data |
There was a problem hiding this comment.
IMHO this is simpler to read, understand and maintain. Great work
dicarobinho
deleted the
afarcasanu/fxios_12363_26946_update_horinzotal_cells
branch
📜 Tickets
Jira ticket
Github issue
💡 Description
Updated horizontal cells based on the latest design
🎥 Demos
Demo
📝 Checklist
@Mergifyio backport release/v120)