Skip to content
This repository has been archived by the owner on Mar 6, 2024. It is now read-only.

Fix Issue 1557 - Fix alignment issues for toggle cells in the settings view #1578

Merged
merged 3 commits into from Nov 21, 2018

Conversation

danielbogomazov
Copy link
Contributor

Fixes issue 1557

Made a new cell similar to the SettingsViewTableAccessoryCell to fix the layout issues of the toggle cells.

screen shot 2018-11-15 at 4 59 18 pm

@sblatz sblatz changed the base branch from search-suggestions to master November 19, 2018 12:42
@sblatz sblatz changed the base branch from master to search-suggestions November 19, 2018 12:42
Copy link
Contributor

@sblatz sblatz left a comment

Choose a reason for hiding this comment

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

One small nit 😄

@@ -416,7 +458,9 @@ class SettingsViewController: UIViewController, UITableViewDataSource, UITableVi
cell.accessibilityIdentifier = "settingsViewController.trackingCell"
cell.accessoryType = .disclosureIndicator
} else {
cell = getToggleCell(indexPath: indexPath)
let toggle = toggleForIndexPath(indexPath)
cell = SettingsTableViewToggleCell(style: .subtitle, reuseIdentifier: "toggleCell", toggle: toggle)
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a lot of code duplication right now (these three lines are repeated thrice) are we able to reduce it somehow?

@danielbogomazov
Copy link
Contributor Author

@sblatz I added a function to make it simpler to setup the cell without having to copy 3 lines whenever making a toggle cell. Let me know if that's what you were looking for :)

Copy link
Contributor

@sblatz sblatz left a comment

Choose a reason for hiding this comment

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

Looks good, thanks for the fix!

@sblatz sblatz merged commit 97259a3 into mozilla-mobile:search-suggestions Nov 21, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants