MobileOrg / mobileorg

MobileOrg iPhone App
http://mobileorg.github.io
GNU General Public License v2.0
556 stars 69 forks source link

add inputAccessoryView to help add markup #263

Closed gitonthescene closed 4 years ago

gitonthescene commented 4 years ago

Hey there,

First of all, thanks for this app. I use it a bunch for taking notes when I'm on the go and syncing it with my org environment.

I'm new to both Swift and to open source contributions, so I appreciate your patience if I've got something wrong.

I guess this is proof of concept, but I think it's pretty functional as is. I'm open to change any of it to conform to style conventions, etc.

I often add notes with URL's when I'm browsing away from my computer. I thought it would be nice if I could create the org link while taking the note instead of when syncing my flagged.org, but typing out the markup is a pain on the phone. This change adds an inputAccessoryView with a link button which converts the highlighted text into an org link much like Ctrl-C Ctrl-L does in org. I also added a key for adding dates marked up with SCHEDULED or DEADLINE and both plain Agenda and non-agenda timestamps.

I also did my own artwork for the keys using inkscape, but I'm fine with changing it.

Please let me know what you think!

gitonthescene commented 4 years ago

To be clear, the idea is you cut-and-paste a url, highlight it and then hit the link key to add the org link markup. The cursor ends up in the "name" portion of the link to allow you to fill that in.

The date key simply replaces the selection or inserts at the cursor position a date of the form you choose.

webframp commented 4 years ago

@gitonthescene Really appreciate the contribution here. I'm ok merging once @dive provides a final sign off too.

gitonthescene commented 4 years ago

@webframp Thanks very much. I have a number of years of experience in financial software, but have only just started poking around Swift. I'd love to pitch in more. Feel free to throw stuff at me.

FWIW, Pythonista3 provides a keyboard that you can add features to, I've been using that to get these features with my MobileOrg and I think it will be a useful addition. Sure you could hand edit the markup on your phone or edit it on your home computer after syncing, but I think a few well chosen features like these make the app experience feel smoother. You don't want to overwhelm users with options, but there might be other auxiliary keys which add value. I don't have any other good ideas at the moment, though.

gitonthescene commented 4 years ago

I'm not sure if it's working around this Apple bug, but this patch on top of the current pull request produces no constraint warnings. Let me know if you think it's worth pulling down.

dive commented 4 years ago

Thanks for the improvements, @gitonthescene! Sorry for the delay with the review, I will try to check it tomorrow, and take a look on this issue with constraints warnings as well.

gitonthescene commented 4 years ago

Thanks. That would be awesome.

dive commented 4 years ago

@gitonthescene, everything looks good to me. I only have a minor comment about the formatting, at some places you put spaces within brackets and at some you do not. It is not critical, as I said, but it is better to be consistent.

About the patch, I see the problem with unsatisfied constraints, and we definitely can go with the patch and replace UIToolbar initializers with the version where we provide a frame, something like this:

let bar = UIToolbar(frame: CGRect(origin: CGPoint.zero, size: CGSize(width: UIScreen.main.bounds.width, height: 8)))

But, in general, I think that all these issues will go away if you replace the implementation based on the UIToolbar with UIStackView with embedded UIButtons. This is a big change and I do not insist. Just let me know if you are not going to make the change, and we will proceed with the current version.

gitonthescene commented 4 years ago

Let's just go with this version for now to get it out and make replacing with UIStackView a separate issue. Just to be clear, I'm going to add the patch to this pull request.

If you prefer I collapse the commits, please let me know.

As for formatting, I normally leave that for an auto-formatter, but I haven't yet set up a formatting hook. Is there a preferred formatting style? Is there a preferred hook to use to do the formatting?

gitonthescene commented 4 years ago

Thanks very much!

webframp commented 4 years ago

Released to TestFlight in build 402