Updates from Koke Toggle Comment Threads | Keyboard Shortcuts

  • Koke 9:57 am on November 8, 2012 Permalink  

    Quick note: if you want to build trunk (starting with r3633), you need to open WordPress.xcworkspace instead of WordPress.xcodeproj

    Otherwise you’ll get this:

    ld: library not found for -lPods
    clang: error: linker command failed with exit code 1 (use -v to see invocation)

    See also: http://ios.trac.wordpress.org/ticket/1473

     
  • Koke 7:19 pm on October 31, 2012 Permalink  

    Some random notes I took while doing bugfixes offline

    Why do we even have global reachability?
    Corner case, but I was trying to add a “localhost” blog to the simulator while offline, and couldn’t do it
    Some users report that they can’t moderate comments in 3.2 since the buttons are disabled. This uses per-blog reachability, but it seems it’s not working ok

    UIAlertView best practices:
    If you need to show an alert:

    • If it’s caused by user action, a local variable is OK
    • If you don’t set a delegate (just show a message), a local variable is OK
    • If the alert is generated as a response to a background event (e.g. AFNetworking failure block) and you set the delegate to ‘self’ because you want custom behaviour: store the alert view as an instance variable, and clear it’s delegate on dealloc

    + [ReachabilityUtils showAlertNoInternetConnectionWithDelegate:]
    I think it’s broken by design: you’re passing an object (usually ‘self’ as a delegate), but you receive no pointer to the object who’s going to have that delegate. So when dealloc happens, it’s impossible to set the delegate to nil

    Since it seems it’s never called from a block, we should be fine, but it still could be way better.

    Instead of passing a delegate and having to implement the alertView:clickedButtonAtIndex: method (BTW, delegate should have type id if that’s expected), the method could be something like – (void)showAlertNoInternetConnectionWithRetryBlock:(void (^)())retryBlock

    We should make a custom WPAlertView to implement “Need Help?”. Most of the code uses [WordPressAppDelegate showAlertViewWithTitle:message: so maybe try to extend that and use it everywhere, since the app delegate is not getting deallocated 🙂

    Move anything not critical to a secondary queue on applicationDidFinishLaunchingWithOptions: (see iTunes connect timeout crash)

     
    • Eric 3:39 pm on November 1, 2012 Permalink | Reply

      +1 for sharing.
      Could you talk a little bit more about the UIAlertView delegate not being able to be set to nil? Since the delegate is assigned and not retained I’m not seeing the issue. Is there something else going on that would make you want to explicitly set the delegate to nil on dealloc?

      I dig the idea to pass a retry block instead of a delegate reference.

      • Koke 3:53 pm on November 1, 2012 Permalink | Reply

        Check http://ios.trac.wordpress.org/changeset/3660/ for instance.

        User refreshes stats on a slow connection. While it’s loading, user switches to a different view and statsWebViewController gets released. The failure block on the network call keeps a reference to “self” so that’s still valid when the network call fails, but gets released (and deallocated) once the block ends. When the user taps “OK” on the alert view, it tries to call the delegate, but it’s not there anymore.

  • Koke 7:08 pm on October 31, 2012 Permalink
    Tags:   

    I’ve just pushed a bunch of commits that should take care of most of the crashes received through iTunes connect, and add stability in general. This was done on a plane, so I’d appreciate someone else looking at these changes and testing them

     
  • Koke 6:30 pm on October 29, 2012 Permalink  

    Big 3.2 crash: r3651-ios should probably take care of it, but we need more user feedback

    Another approach to this: https://github.com/koke/wordpress-ios/commit/f1a06916bfdc82fe7f0dff2cc44b9c7322e6980d
    Instead of just eating up whatever XML-RPC returns, we convert it to the data type we expect, or otherwise set it too nil. Do you think that’d be useful or is it too much?

     
    • Eric 8:08 pm on October 29, 2012 Permalink | Reply

      I like that approach linked to on github. Its seems more robust.

  • Koke 1:06 am on October 19, 2012 Permalink
    Tags: 3.3, ,   

    I’ve been modernising the codebase and updated libraries: modernize-3.3, cocoapods

    • Migrated to ARC
    • Started using CocoaPods
    • Replaced some obsolete code (performSelector: and friends)

    Please, don’t commit to trunk until this is merged, or I’ll spend a week crying in front of conflicts 😉 Use patches, local branches, push to github,… or just ping me to merge this

    A few things I want to do before merging:

    • More testing (this is the only real blocker, the next can happen on trunk)
    • Merge our xmlrpc streaming changes with the new streaming upstream branch, and get XMLRPCDataCleaner into master
    • Consider dropping Facebook SDK for iOS5 and use the iOS6 Facebook framework
    • Remove viewDidUnload methods (deprecated in iOS6)
     
  • Koke 12:18 pm on October 3, 2012 Permalink  

    In #1327-ios, I’ve bumped the “required” version to 3.1 instead of 3.0. It’s just a warning, so older sites would still work the same as they did before.

    Should we require a higer version? Why not just the latest stable?

     
    • Isaac Keyet 4:14 pm on October 3, 2012 Permalink | Reply

      Since we’ve traditionally been fairly loose on the restrictions we should gradually move towards it instead. Running an old version of WP is of course a security threat, but we still only need a certain subset of the API that may be available in versions prior to the latest stable.

      To avoid backlash from users, I think we should gradually move towards better restrictions. Maybe we could even add an update function in the apps…

    • Dan 5:10 pm on October 9, 2012 Permalink | Reply

      We could match it up with Jetpack, I was just testing the app on WP 3.1 and Jetpack showed a “Jetpack requires WordPress version 3.2 or later.” prompt when trying to activate it.

  • Koke 12:43 pm on June 26, 2012 Permalink  

    Un-sticked the previous todo list for 3.1, using trac’s 3.1 milestone now

     
  • Koke 10:26 am on June 26, 2012 Permalink  

    As a general rule, you shouldn’t need to use SFHFKeychainUtils directly. If you need a blog’s password you can use [blog fetchPassword]

     
  • Koke 2:47 pm on June 18, 2012 Permalink  

    Here’s the list of outstanding issues for 3.1 that I’ve found after testing the app.

    Besides discussing the mockups and implementing the style changes:

    Most likely I’ve missed a bunch of things, so keep adding things that don’t work as you would expect, but no new features please. Let’s focus on getting what’s there “feel right” first.

     
    • Dan 4:14 pm on June 18, 2012 Permalink | Reply

      • Isaac Keyet 2:25 am on June 19, 2012 Permalink | Reply

        I’ll look into simplified comps to ease development in this first iteration, but yes. Let’s start with implementing a new default.png 😉

    • Dan 8:14 am on June 19, 2012 Permalink | Reply

    • Jorge Bernal 11:59 am on June 19, 2012 Permalink | Reply

      See https://img.skitch.com/20120619-j41t9f235guxq3w2utj9raa3ag.jpg

    • Dan 3:39 pm on June 19, 2012 Permalink | Reply

      https://img.skitch.com/20120619-1d7niu1rtyehe14xh78fb4axm5.png

    • Dan 3:42 pm on June 19, 2012 Permalink | Reply

    • Jorge Bernal 5:08 pm on June 19, 2012 Permalink | Reply

      Since our user agent is wp-iphone, even for the web browser on iPad, some sites (like TechCrunch) are serving us mobile optimized content. Maybe it’s time to switch it to wp-ios, or at least wp-ipad for the iPad

    • Brad 8:59 pm on June 19, 2012 Permalink | Reply

    • Dan 12:28 pm on June 20, 2012 Permalink | Reply

    • Dan 1:39 pm on June 20, 2012 Permalink | Reply

      Selected comment cells have some height problems

      @kokejb – I can’t replicate that one.

    • Jorge Bernal 5:03 pm on June 20, 2012 Permalink | Reply

    • Brad 5:33 pm on June 20, 2012 Permalink | Reply

    • Brad 6:20 pm on June 20, 2012 Permalink | Reply

      • Brad 6:35 pm on June 20, 2012 Permalink | Reply

        only seems to happen when the table view is scrolled up and down a couple times

        • Eric 2:44 pm on June 22, 2012 Permalink | Reply

          Was this on the web-based stats or the previous version?

          • Brad 2:51 pm on June 22, 2012 Permalink | Reply

            Previous table view version. It seems to be alright now. I’ll check it off.

    • Dan 10:33 am on June 21, 2012 Permalink | Reply

    • Brad 2:58 pm on June 22, 2012 Permalink | Reply

    • Jorge Bernal 12:20 pm on June 25, 2012 Permalink | Reply

    • Jorge Bernal 12:27 pm on June 25, 2012 Permalink | Reply

    • Brad 8:05 pm on June 25, 2012 Permalink | Reply

    • Brad 8:38 pm on June 25, 2012 Permalink | Reply

      No way to access Reader’s topic list on iPad.

      Would a modal view be the best UI for this? Not sure where to put the button, though. Maybe on the far right side of the sidebar item for “Read” if on iPad?

      • Jorge Bernal 9:43 am on June 26, 2012 Permalink | Reply

        I think the topic selector button should be in the reader panel somehow, not in the sidebar

  • Koke 1:02 pm on May 31, 2012 Permalink  

    Just submitted 3.0.1 to the App Store, so no more commits on branches/3.0.1

    When it’s approved I’ll move it to tags/3.0.1

     
c
Compose new post
j
Next post/Next comment
k
Previous post/Previous comment
r
Reply
e
Edit
o
Show/Hide comments
t
Go to top
l
Go to login
h
Show/Hide help
shift + esc
Cancel