-
-
Notifications
You must be signed in to change notification settings - Fork 69
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
add inputAccessoryView to help add markup #263
Conversation
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. |
9d4df4b
to
42407cb
Compare
4db4ad4
to
c6a50bc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the contribution, @gitonthescene! You have mentioned that you are new to Swift, so, I hope that my comments will help you to understand the code better. Feel free to ask any questions you have.
let scheduleButton = UIBarButtonItem( title: "Schd", style: .plain, target: self, action: #selector(insertDateSchedule) ) | ||
let deadlineButton = UIBarButtonItem( title: "Dead", style: .plain, target: self, action: #selector(insertDateDeadline) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not sure about these abbreviations, I understand that they will not fit otherwise but still. @webframp, what do you think?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't have any good suggestions for alternatives but agree it doesn't quite fit for me. I think I'm probably be fine with them unless we have suggestions for clearer alternatives.
bc8849d
to
9c3d534
Compare
@gitonthescene Really appreciate the contribution here. I'm ok merging once @dive provides a final sign off too. |
@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. |
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. |
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. |
Thanks. That would be awesome. |
@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 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 |
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? |
Prefer guard for style consistency
e188bd4
to
216ebba
Compare
Thanks very much! |
Released to TestFlight in build 402 |
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!