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

iOS: three column layout on iPad #817

Closed
Dejal opened this issue Nov 14, 2015 · 27 comments
Closed

iOS: three column layout on iPad #817

Dejal opened this issue Nov 14, 2015 · 27 comments

Comments

@Dejal
Copy link
Collaborator

Dejal commented Nov 14, 2015

Idea: the web app has a three-column layout (feeds, detail, story). This could be done in the iPad app too, perhaps with a narrow column on the left side, just wide enough for icons, which expands to show the full width when tapped, perhaps via a slide-over animation, or making the second column narrower till an item is tapped.

On iPad Pro, there's enough room for full width of all three columns.

@Dejal
Copy link
Collaborator Author

Dejal commented Mar 30, 2020

Transferring from #1247, @samuelclay wrote:

On the three-pane, I would love to rethink how NewsBlur iOS app has the panes. Today it's not great, but it works. I would be happy to move to something a bit more like sliding doors (ala Reeder), although that would make the dashboard river harder to show.

I can't find a video of how Reeder works, but I can imagine. There are certainly a number of ways it could be supported, which could be useful for both iPad and Mac.

The easiest would be a control like on the NewsBlur website to choose between the current two-pane view and a three-pane view. Probably iOS would default to two-pane, and macOS would default to three-pane.

Animation could be used when switching modes, or moving between views.

A lot of the layout issues we've had with iPad is due to the way the container view controller lays out views on top of each other. Going for more of a sliding approach, with three panes side-by-side-by-side, and just resizing or sliding off screen, might make that tidier.

We could also consider moving to a standard UISplitViewController, instead of a custom container, which would give standard behaviors and size class support automatically. To do three-pane, we could embed a second UISplitViewController in the detail view.

@samuelclay
Copy link
Owner

We started with a splitviewcontroller back in 2012 and abandoned it because it didn't fit our needs for story titles on left vs on right. I'd be happy to go back to it, as the current layout is broken on catalyst and is wonky on iPad sometimes when returning to the app (although that's been fixed, it keeps popping back up).

I believe this work should come before catalyst as it's needed to improve catalyst.

@Dejal
Copy link
Collaborator Author

Dejal commented Mar 30, 2020

Yeah, using Apple's APIs is usually better than rolling our own, though not always.

I think you meant to say story titles on left vs bottom, which is a good point; that wouldn't be automatically supported with a split view controller, though could still be done in much the same way as now, just within the detail pane. (It's a pity that UIKit's split view is less flexible than AppKit's; the latter supports more than two panes, plus vertical orientation in addition to horizontal. But we can make it work.)

I'll investigate further to decide on the best approach, but I'm leaning towards going with UISplitViewController if feasible.

I agree that this is a prerequisite for Catalyst, so I'll do this first.

@Dejal Dejal added high and removed low labels Mar 30, 2020
@Dejal
Copy link
Collaborator Author

Dejal commented Apr 24, 2020

I've got a working prototype using nested UISplitViewController instances, and I think that'll work nicely. I've written up a bunch of notes, and will mull it over for the next few weeks, and implement it next month, other issues permitting.

@samuelclay
Copy link
Owner

Excellent, I'm looking forward to this!

Dejal added a commit that referenced this issue Aug 30, 2020
- Fixing warnings.
- Adding Swift subclasses of ObjC ones.
Dejal added a commit that referenced this issue Aug 30, 2020
- Numerous changes for three-column layout support and other projects.
- Work in progress.
@Dejal
Copy link
Collaborator Author

Dejal commented Aug 30, 2020

Work on this is underway!

I have made extensive changes to the app in the Catalyst branch to modernize the app to support multiple projects: three column layout, plus of course Catalyst (#1247) to run on a Mac, multiple windows (#1264), vertical scrolling gestures (#1351), better full-screen operation (#1338), and modern state restoration (#1304), which may also provide Handoff support, and more.

One of my goals is to eliminate some unnecessary platform-specific functionality, since the app running on an iPad can be put into split view and work like on iPhone, and of course to support Mac too via Catalyst. I also want to eliminate custom layout stuff, since that is the primary source of drawing bugs — so using a standard UISplitViewController instead of a custom container (with works well on iPhone too, falling back to push and pop navigation), and a standard UIPageViewController instead of arranging views in scroll view, and the resulting mess with fullscreen and such.

This is still a work in progress; the app in this branch is currently in a rather disassembled state, with unfinished parts (e.g. the stories are currently missing), and several known bugs. But it already supports three columns (options for two columns will be added later), and other infrastructural changes to support all of those projects.

Note that building this requires Xcode 12 (currently in beta), and three columns are only supported on iPadOS 14 and macOS 11. When I'm done, it'll work in two-column mode on iOS 13, but that isn't finished yet. (iPadOS 14 introduced native support for three column split views, so I didn't need to do my less ideal prototyped solution. Native support is much better.)

If you try it, don't bother reporting issues yet; I have a list. I'll tag the related GItHub issues as I work on each aspect, though some work will benefit multiple issues.

Here's a screenshot for your enjoyment:

Simulator Screen Shot - iPad Pro (12 9-inch) (4th generation) - 2020-08-29 at 21 08 58

@samuelclay
Copy link
Owner

Lovely! My advice is don't bother with iOS 13 support, as people who don't upgrade can still use the older version of the app. And a rewrite is a good time to force an upgrade to latest.

@Dejal
Copy link
Collaborator Author

Dejal commented Aug 31, 2020

Fair enough, that'll make it simpler. I assumed you wouldn't want to drop older OS versions, but you're right that it's reasonable to require it.

It'll be an exciting update!

Dejal added a commit that referenced this issue Sep 27, 2020
- Fixing deprecations.
Dejal added a commit that referenced this issue Sep 27, 2020
- Fixed deprecations.
Dejal added a commit that referenced this issue Sep 27, 2020
- Horizontal and vertical page view controllers, using experimental views.
- This will help with #1351 (gestures in vertical scrolling).
- Changes preparing for #1264 (multi-window) support.
- Various other tweaks.
Dejal added a commit that referenced this issue Sep 27, 2020
- More progress on the story pages stuff.
Dejal added a commit that referenced this issue Sep 27, 2020
@Dejal
Copy link
Collaborator Author

Dejal commented Sep 27, 2020

More progress; the stories are now appearing (in a somewhat broken way), and supporting scrolling horizontally or vertically (for those two options, plus the #1351 gestures in vertical scrolling request). Still lots to do, and too broken to be worth playing with yet, but getting there.

Here's a screenshot:

Simulator Screen Shot - iPad Pro (12 9-inch) (4th generation) - 2020-09-26 at 21 21 42

Dejal added a commit that referenced this issue Oct 31, 2020
- Lots more progress on the detail view and other areas.
@Dejal
Copy link
Collaborator Author

Dejal commented Oct 31, 2020

Lots more progress. The stories are working pretty well now. Still some issues, e.g. first time use, and I haven't tested iPhone yet, but getting there. It could be decent enough to try if you like. You can swipe between stories vertically or horizontally (at the same time, for #1351), collapse one or both of the sidebars, etc.

Here's another screenshot:

Simulator Screen Shot - iPad Pro (12 9-inch) (4th generation) - 2020-10-30 at 20 44 40

@samuelclay
Copy link
Owner

Wow that's an amazing experience. I can see this is going to be transformative for the app. Nice work! Greatly looking forward to getting this on iPhone and in portrait mode on iPad.

@Dejal
Copy link
Collaborator Author

Dejal commented Jan 12, 2021

I plan to work on it later this week. I'm aware of a bunch of bugs, but if you like you can list the most egregious ones, to make sure I address them. Just mention them in comments here, rather than in new issues.

Dejal added a commit that referenced this issue Jan 17, 2021
- Scrolling story instead of pages.
- Change pages vertically only when at the top or bottom of a story.
Dejal added a commit that referenced this issue Jan 17, 2021
- Fixed sporadic crash when changing pages.
Dejal added a commit that referenced this issue Jan 17, 2021
- Fixing several issues, including crash on launch due to loading the feed detail multiple times.
Dejal added a commit that referenced this issue Jan 17, 2021
- When rotating iPhone from portrait to landscape, the feed detail now appears correctly.
- Eliminated some popping; changing columns works differently with the split view.
@Dejal
Copy link
Collaborator Author

Dejal commented Jan 17, 2021

A bunch more fixes and improvements, including the vertical scrolling issue and crashers.

I'll do more in a couple of weeks, but if you deem it good enough, you could do a TestFlight now. Either way, feel free to comment with bugs.

@nriley
Copy link
Contributor

nriley commented Jan 17, 2021

In the dark theme there are a couple of stray bright dots and the bottom bar in the feeds list seems to need to be a bit wider.

qdgvnhl1QVqPuEUJbE9EbA_thumb_320c

Swiping horizontally between articles doesn't work immediately when you launch the app on iPad; tapping the full screen story view (double arrow) button makes it work, then you can exit the full screen and continue to swipe horizontally.

Otherwise, so far seems a lot smoother/less buggy.

(Deleted my prior comment; it is still the case but doesn't actually seem to be new with this branch.)

@nriley
Copy link
Contributor

nriley commented Jan 18, 2021

Full-width images are sometimes offset so they're off the right side of the screen, then sometimes they aren't full-width either.

I can't get the tap-hold-on-image behavior to display alt text and zoom to activate any more.

@nriley
Copy link
Contributor

nriley commented Jan 18, 2021

None of the keyboard shortcuts to get back to the dashboard from the story list are working.

Thinking about this more carefully, the dashboard really isn't a separate view any more, so maybe those shortcuts should do the same thing as 'show/hide sidebar'?

As someone who exclusively browses in All Stories, the only reason I ever go back to the dashboard is to refresh the story list. Showing the sidebar then using "Open All Stories" does the same thing in the new version.

Finally, noticing that the story view doesn't slide smoothly from one story to another any more; the view flashes instead.

@samuelclay
Copy link
Owner

Thanks for getting those updates out, I'm now on the latest build again. It's looking better!

I also am missing the sliding action when tapping next and previous. It helped with getting my bearings around a feed as I go back and forth. Would be best to replicate the old behavior of sliding over a space equal to the distance (# of reads before an unread) between stories.

On my iPhone SE I find that I often can't scroll a story after loading it. And both vertical and horizontal scrolling are enabled, which feels odd. We should turn the other off.

Also, often when scrolling and I lift my finger, the full-screen mode is disabled. I'd like to have full screen mode not enabled on a tap. I seem to recall you wanted that tap in there, but at the very least it should not fire as often as it does.

Lastly, I'm seeing a "You must be premium" notice on the story list pretty regularly when I first go into the app. I use the dashboard widget mostly and it's been throwing me for a loop and just opening to a blank story list. It's a bit orthogonal to this work, but it's in the same code paths as story layout.

Dejal added a commit that referenced this issue Jan 31, 2021
- Since the UIPageViewController approach to paging had multiple issues that were proving impossible to work around, I decided to revert to the old UIScrollView-based approach.
Dejal added a commit that referenced this issue Jan 31, 2021
- Restored the old scroll direction preferences, since I’ve removed the UIPageViewController approach (see #817).
- I’ll come back to this feature enhancement later, with a different solution.
Dejal added a commit that referenced this issue Jan 31, 2021
- Story pages view controller appearance tweaks.
Dejal added a commit that referenced this issue Jan 31, 2021
Dejal added a commit that referenced this issue Jan 31, 2021
Dejal added a commit that referenced this issue Jan 31, 2021
- More iPhone & iPad fixes.
- Fullscreen fixes.
- Disabled the tap to toggle fullscreen.
- Restored the old-style fullscreen toggle by pulling down when at the top.
@Dejal
Copy link
Collaborator Author

Dejal commented Jan 31, 2021

While I was redoing the infrastructure of the app to support the three column layout, I took the opportunity to implement a solution for #1351 at the same time, using nested UIPageViewController instances. But that proved to be rather problematic, as it wasn't an ideal fit, and I had to try to hack the framework to get it to cooperate, having multiple issues that were proving impossible to work around. You also didn't seem to like the main benefit of that, being able to page horizontally and vertically at the same time.

So, I decided to revert to the old UIScrollView-based approach, which I've now done. You can swipe between stories in the same way as always, and the scroll direction preferences are back.

I also disabled the tap to toggle fullscreen, and changed it to hide the nav bar when scrolling, and show when pulling down at the top.

Plus, I fixed a bunch of issues on iPad and iPhone with the three-column layout. That is working rather well now.

I think the app is in better shape now. Still a few minor issues and cleanup to do, but feel free to kick the tires.

Dejal added a commit that referenced this issue Feb 28, 2021
- Fixed not showing feed detail content for feed when first launched on iPhone.
@nriley
Copy link
Contributor

nriley commented Feb 28, 2021

Updated to your latest version. Do you want me to start filing separate issues on these?

New issues I noticed:

Old issues (are you able to reproduce these?)

@Dejal
Copy link
Collaborator Author

Dejal commented Feb 28, 2021

Yes, please add separate issues for problems with the Catalyst branch (including those), using the title prefix "iOS Catalyst:", and tags "iOS" and "high" or "medium" as appropriate. Anything that's a regression should be high priority.

I'll close this issue once I've cleaned up the code to remove redundant stuff, which I'll do once I've fixed any remaining problems.

We're getting there!

@samuelclay
Copy link
Owner

Agreed, this build is great. Would you prefer I release a TestFlight now or wait until you've had a chance to run through some of the tickets @nriley filed?

@Dejal
Copy link
Collaborator Author

Dejal commented Mar 1, 2021

Up to you. I just had a quick look at those issues, and none of them seem fatal problems, so you could do a TestFlight if you like. Note that I will next be working on NewsBlur in the week ending March 27 (I've got some other stuff I need to do in the meantime), so you could treat this time as an opportunity to gather more issues via TestFlight, or might not want to leave people waiting for updates that long.

(And if anyone else wants to contribute fixes to any of @nriley's issues, feel free; there's more than enough to keep me busy when I do resume.)

@nriley
Copy link
Contributor

nriley commented Mar 1, 2021

Only one of them is a crasher (#1426); the rest are just cosmetic/annoying. Perhaps worth adding to the TestFlight release notes as known issues so we don't get duplicate reports. (Wish I had more time to work on NewsBlur right now but I don't.)

Dejal added a commit that referenced this issue Apr 24, 2021
- Fixed issues with the feed detail when rotating a Plus/Max phone to landscape.
Dejal added a commit that referenced this issue Apr 24, 2021
- Fixed displaying popovers on iPhone in landscape on Plus/Max devices.
Dejal added a commit that referenced this issue Apr 25, 2021
- When using top or bottom layout on iPad, now includes the feed detail menu in the nav bar.
- Fixed an empty middle column when switching from top or bottom layout to left layout.
Dejal added a commit that referenced this issue Apr 25, 2021
- Removed the obsolete container view.
- Tweaked a few places that still used it.
Dejal added a commit that referenced this issue Jun 6, 2021
- Cleanup and removal of obsolete code and resources.
@Dejal Dejal closed this as completed Jun 6, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants