Some scattered high-level feedback:
I like the definition of the menus as very simple, mostly declarative Swift code. Action definition is kinda thorny! Two particular things come to mind: Firstly, an action probably wants some input into what happens after the action is triggered. For example, when we have bookmark 'foldering', the bookmark star might be a two-step process, and we might want to keep the menu up until the user has picked a folder. If the operation fails, the menu should stay up. Otherwise, the menu should close. There are a bunch of ways to do this kind of thing. One is to rely on side-effects — e.g., have the action take the menu as an argument, and have the action be responsible for dismissing the menu either before (when we open settings?) or after (bookmark) it does its own work. Secondly, actions can be async and can fail. Perhaps one consequence of this is related to your animation concern: when I tap the bookmark star, the item might not end up bookmarked, but if it does it'll be 50-100msec later, at which point we want to animate the state of the menu item (blue star). So the menu triggers the action, and the action pokes the menu, perhaps even switching out menu items. Right now actions seem to be one-shot synchronous (or no handled result) calls; this is a good time to think about if the 'signature' needs to be different. I have no good suggestions about how to make sure that actions and the menu interact well visually, beyond letting the action own everything — I'm thinking about what happens when the menu triggers Find In Page, and we want those two animations (menu dismissal, find in page bar appearance) to coordinate. It's worth noting that there are concurrency worries here. The state of menu items depends on async operations (isBookmarked), but the menu or even the current tab could change while those operations are running. On Wed, Mar 2, 2016 at 9:26 AM, Emily Toop <et...@mozilla.com> wrote: > [Discussion moved to public channel for openness] > > iOS folks, > > After last weeks meeting with Steph & James, we decided to try a different > approach with the menu and to try out a table view/ collection view style > datasource/delegate pattern with the things that the menu items perform > encapsulated as Actions that could be called from anywhere inside the > codebase (i.e. so the same Action can be used to create a new tab from > anywhere in the app). > > You can find an updated version of the sample project code with this > pattern here: > https://github.com/fluffyemily/firefox-menu-experiments/tree/actions-datasource-delegate > (the original implementation idea can be found on the master branch of this > repo). > > Things I have not managed to figure out in here yet - > * how to perform custom animations on menu item selection. I feel this > should be part of the menu item code somewhere, maybe a function passed in > like MenuItemState.isVisible but it feels a bit messy. I'd like to keep > specific behaviour like this encapsulated though so it's not spread > throughout the project). > > I will arrange another meeting after the iOS team meeting tomorrow so > anyone interested can come and discuss this & feedback, but also feel free > to feedback here. > > Emily > > On 24 February 2016 at 18:39, Emily Toop <et...@mozilla.com> wrote: > >> So, I've created a little project to explore some of my ideas around the >> menu and how it would work. I was hoping to get it more finished by the end >> of today (i.e. now) but things haven't quite worked out that way. So if you >> can ignore the fact that it's extremely colourful and the buttons don't >> work just yet, it still demonstrates where I've got to with thinking about >> this. >> >> I'm going to arrange a meeting for after the iOS team meeting tomorrow to >> discuss this, so if you're interested, stick around after the team meeting >> and we can go deeper into this. >> >> E >> >> On 23 February 2016 at 18:43, Steph Leroux <sler...@mozilla.com> wrote: >> >>> I recently did a refactor of Settings for the Touch ID/Passcode work >>> I've been doing since I wanted to re-use the settings for configuring >>> passcode stuff [1]. I probably wouldn't do as much subclassing for the menu >>> options though and stick with one-level protocol implementations. Also, I >>> can see the logic associated with each option being unique to potentially >>> large so I could see that logic encapsulated in a 'Controller' or 'Action' >>> protocol that each menu option could have a reference to. >>> >>> [1] >>> https://github.com/mozilla/firefox-ios/blob/master/Client/Frontend/Settings/AppSettingsOptions.swift >>> >>> On Tue, Feb 23, 2016 at 12:50 PM, James Hugman <jhug...@mozilla.com> >>> wrote: >>> >>>> I would definitely want to separate the specifying the menu items in a >>>> separate file, away from the menu, so that non-devs could cut and paste new >>>> menu items. >>>> >>>> MenuConfig.swift >>>> MenuItemX, MenuItemY.swift, MenuItemZ.swift >>>> MenuVC.swift >>>> >>>> This is specifically not like current Settings which is all mixed up in >>>> the TableVC. Personally, I'd like to see Settings refactored to be more >>>> like this pattern. >>>> >>>> On Tue, Feb 23, 2016 at 3:11 PM, Emily Toop <et...@mozilla.com> wrote: >>>> >>>>> The idea behind the plist was that, by making it human readable by >>>>> others outside of swift developers, simple changes like icons, titles or >>>>> ordering within the menu could be tweaked by others, i.e. Robin or Sevaan, >>>>> without them having to touch the code. Having the configuration in code, >>>>> as >>>>> in Settings, is definitely another alternative that I considered. It would >>>>> be cleaner, sure, but less accessible. Is that accessibility something >>>>> that >>>>> we should consider? >>>>> >>>>> On 23 February 2016 at 14:00, Steph Leroux <sler...@mozilla.com> >>>>> wrote: >>>>> >>>>>> The idea of this would be so that each menu item would be nicely >>>>>>> encapsulated, easily tested through dependency injection/mocks, isolated >>>>>>> (as much as possible) from the BVC and the entire menu would be very >>>>>>> easily >>>>>>> configurable, maintainable and extendable.\ >>>>>>> >>>>>> >>>>>> +100. I agree with James though - I don't think having it in a .plist >>>>>> provides us with much benefit other over Swift for building up these >>>>>> items. >>>>>> It seems like a similar approach that we take with the settings menu >>>>>> where >>>>>> each setting contains it's relevant logic. As far as setting up the >>>>>> protocols go, I think taking a chapter out of Richard's book with how he >>>>>> did sync would be the way to go. Seems like every type is declared as a >>>>>> protocol first then implemented as class/struct where needed to allow us >>>>>> to >>>>>> create mock versions in the test bundle since we don't have a mocking >>>>>> library. I'd love to talk more about this during the iOS meeting this >>>>>> week >>>>>> to throw around some ideas. >>>>>> >>>>>> On Tue, Feb 23, 2016 at 8:18 AM, James Hugman <jhug...@mozilla.com> >>>>>> wrote: >>>>>> >>>>>>> I think this is great idea, right up to putting it in to a plist >>>>>>> (but only because it's a small amount of XML replacing a small amount of >>>>>>> Swift, and the configuration parser will need to be written). >>>>>>> >>>>>>> >>>>>>> >>>>>>> >>>>>>> On Tue, Feb 23, 2016 at 11:39 AM, Emily Toop <et...@mozilla.com> >>>>>>> wrote: >>>>>>> >>>>>>>> Hey, >>>>>>>> >>>>>>>> I'm not sure if you've seen the new menu proposals. If you haven't >>>>>>>> here are the iPad and iPhone portrait designs so far ( >>>>>>>> https://mozilla.invisionapp.com/share/7Z647TWG4#/screens, >>>>>>>> https://mozilla.invisionapp.com/share/4363MPDYG#/screens). >>>>>>>> >>>>>>>> I've been thinking that I would like to add this menu in a way that >>>>>>>> is easily configurable, so that adding, removing and editing menu >>>>>>>> items is >>>>>>>> really simple. >>>>>>>> >>>>>>>> I am imagining something along the lines of a plist/configuration >>>>>>>> file containing an icon, title, priority and the name of a class for >>>>>>>> each >>>>>>>> item. The class would have to confirm to some prototcol that defined >>>>>>>> everything that was required to determine if the menu item should be >>>>>>>> displayed and what actions it could perform, including any states and >>>>>>>> animations. Each class would be dynamically loaded by the menu via >>>>>>>> introspection. >>>>>>>> >>>>>>>> The idea of this would be so that each menu item would be nicely >>>>>>>> encapsulated, easily tested through dependency injection/mocks, >>>>>>>> isolated >>>>>>>> (as much as possible) from the BVC and the entire menu would be very >>>>>>>> easily >>>>>>>> configurable, maintainable and extendable. >>>>>>>> >>>>>>>> I guess I have 2 questions. >>>>>>>> >>>>>>>> 1. Do you think this is a good idea? >>>>>>>> 2. If not, what other ideas do we have so that we can make this >>>>>>>> menu avoiding the Massive View Controller problem and a whole bunch of >>>>>>>> hard >>>>>>>> to maintain spaghetti code? >>>>>>>> >>>>>>>> If you're interested, can we get together and figure out between us >>>>>>>> how the configuration file and protocol should look/the behaviour it >>>>>>>> should >>>>>>>> have? I figure a group design of the framework for this would help >>>>>>>> identify >>>>>>>> all the things that would need thinking about and spread the knowledge >>>>>>>> of >>>>>>>> how it works making review for what is looking like quite a large >>>>>>>> feature, >>>>>>>> much easier. >>>>>>>> >>>>>>>> Thoughts appreciated. >>>>>>>> >>>>>>>> Emily >>>>>>>> >>>>>>> >>>>>>> >>>>>> >>>>> >>>> >>> >> > > _______________________________________________ > mobile-firefox-dev mailing list > mobile-firefox-dev@mozilla.org > https://mail.mozilla.org/listinfo/mobile-firefox-dev > >
_______________________________________________ mobile-firefox-dev mailing list mobile-firefox-dev@mozilla.org https://mail.mozilla.org/listinfo/mobile-firefox-dev
