duckduckgo / ios-search-and-stories

DuckDuckGo Search & Stories for iOS
Other
176 stars 79 forks source link

Padding issue on iPad settings #125

Closed thm closed 8 years ago

thm commented 9 years ago

photo 01-10-15 9 22 05 pm

sreilly commented 9 years ago

@thm Is the issue here that the indentation (on both sides) is too large, and should be an absolute value rather than relative to the overall width?

thm commented 9 years ago

@sreilly yes

sreilly commented 9 years ago

@thm sorry for the lack of progress on this one. I've been trying different approaches for nearly the entire day. This is a built-in behaviour of the UITableViewCell that has turned out to be very difficult to override. We can replace all of the cells/rows with our own views but that will take a little while to implement, and likely not look great the next time Apple updates their overall look. Would you like me to spend more time to try and replace the built-in list rows or should we go with the system default behaviour? I'll continue to spend a bit of (my own) time to try and find a better approach.

thm commented 9 years ago

@sreilly I didn't know that, I thought it looked alright before. Would it be an easier alternative to use a proper split-view here?:

quick-mockup

sreilly commented 9 years ago

That's a fantastic idea. On it!

On 5 Oct 2015, at 19:16, Thom notifications@github.com wrote:

@sreilly I didn't know that, I thought it looked alright before. Would it be an easier alternative to use a proper split-view here?:

— Reply to this email directly or view it on GitHub.

sreilly commented 9 years ago

@thm yet again this ends up not being as straightforward as I had hoped. The iOS master-detail view won't work with the push-stack controls like with the rest of the app. Basically, the split view needs to work at the root level, so it effectively refuses to be used with the search bar at the top and tab bar at the bottom. I'll see what else I can try, including using a third-party split view controller. I've tried one of them, but it's a bit old and so would need some tweaking before it'll work properly.

sreilly commented 8 years ago

@thm ok, the latest build includes a custom split view adapted from the open source MGSplitViewController. Can you give it a go on an iPad and let me know how it feels and if you'd like any tweaks to it?

thm commented 8 years ago

Thanks, the only thing left to be fixed is the weird shifted dividers inbetween elements. On the Favorites > Searches page the dividers are aligned with the icons correctly, but on the Recents > Searches page they're way off. Same goes for the Settings > Sources page. Moving to the 'iteration 1' milestone though...

sreilly commented 8 years ago

@thm these are showing up aligned for me on the iPhone and iPad. Can you confirm whether it's still an issue? If it is, would you be able to send a screenshot of the problem? Thanks!

thm commented 8 years ago

Sure thing, on both iPhone and iPad I currently see this on the Settings > Sources page: alignments-1

And this is just on iPad (on closer inspection I also notice a weird extra line above the favorite searches list): alignments-2

SwiftlyDeft commented 8 years ago

@thm Good new's, sorted out the Recent Searches padding. Have a look at this screenshot:

simulator screen shot 8 01 2016 2 05 44 pm

You'll be able to see and verify the fixes in the next build update. The extra spacing in the Favorite searches has also been dealt with

SwiftlyDeft commented 8 years ago

@thm Regarding the concern about alignments for the Settings. Have a look at the attached screenshot from the Settings App on iOS.

screen shot 2016-01-08 at 2 16 16 pm

Notice that T and V are flushed left (edges meet) but with F theres a little space. It's the text rendering. I also checked this out on the mail app.

thm commented 8 years ago

@SwiftlyDeft sorry for the confusion, the two purple guides were meant to compare the dividers with the text. The iPad screenshot looks good! guides

SwiftlyDeft commented 8 years ago

@thm, yikes, what a mistake. Sorry about that, i'll get this fixed in the next build. Thanks for clarifying the issue

SwiftlyDeft commented 8 years ago

@thm, Fixed, the title, subtitle and separator inset's now all align. This will be in the new build. Thank you