disable swipe if not mobile | adjusted swipe response

[markamaze]

Here is what I have regarding fix #586.

I used the isMobileBrowser tool, injecting it into Entry since it contained other props being set to disable tools. Carried that through to Header and used it in handleDragStart to avoid setting state.dragStart* values.

I decided to add buttons for removing header and advancing todo state in the HeaderActionDrawer with handlers and a flag passed in from the Header. Not sure if that is wanted but it was good practice for me and seemed reasonable. Would need feedback on icons and placement if it should stay.

I also multiplied the *_ACTIVATION_DISTANCE values by three and it seems like it might be a bit better regarding unintentional swipes/deletes/state changes. I thought increasing both would help to avoid initiating free drag when scrolling and making sure you want to delete or change state by increasing the swipe activation distance.

*I am questioning now naming the prop passed in from Entry, "shouldDisableSwipe", as it is set by isMobileBrowser which may be desirable to use in other situations within OrgFile in the future?

[markamaze]

I'm guessing I was supposed to use the contrib branch and not develop, or master? Not sure what went wrong

[tarnung]

@markamaze I have yet to run your changes. So I'll comment on them later.

About the git branches: We only recently added the develop branch for bigger features that take a while to complete. Currently my code to support viewing the content of several files in agenda, search etc. is on there. It's ok to make smaller pull requests against master, since we don't want to block them until the next big feature is ready.

So you can either rebase your changes to master or leave them on develop where it will take a while longer for them to 'go live'.

[markamaze]

Tried to rebase it but got conflicts. Won't have internet access on my computer for a couple days anyways so I'll just leave it for now.

Thanks for clarifying my confusion with the branches!

[tarnung]

@munen what is the best way to check out the code of a pull request that's on someones fork?

Do you clone their repository separately? Do you add their repository as an additional remote?

[munen]

@tarnung If you're asking for 'the best way', I'm personally using:

• https://magit.vc/
• https://github.com/magit/forge

With this setup, it doesn't matter if I work on 'my' PR, on another persons, on a fork, on code, on an issue, on Github, on Gitlab, it's all nicely unified. Magit alone is one of those tools that some people that it's singlehandedly valuable enough to use Emacs for.

With stock git, I'd follow the "view command line instructions" from Github:

Step 1: From your project repository, check out a new branch and test the changes.

git checkout -b markamaze-issue/586 develop
git pull https://github.com/markamaze/organice.git issue/586

Step 2: Merge the changes and update on GitHub.

git checkout develop
git merge --no-ff markamaze-issue/586
git push origin develop

[markamaze]

My apologies for having not read the contribution guidelines before doing this pr. I see where I went wrong. Lots to learn. Would be glad to correct it if it helps.

[munen]

My apologies for having not read the contribution guidelines before doing this pr. I see where I went wrong. Lots to learn. Would be glad to correct it if it helps.

Sure, please do(;

Nobody has to get it right on the first try^^

I'll convert this into a draft PR. You can put it into 'ready for review' whenever you're ready for review, again.

[markamaze]

Not sure if I'm a fan of using two more buttons in the header bar. Especially when they look like this

I kinda suspected the buttons may not be wanted, I only added them as placeholders for my own benefit and decided to include them in what I pushed in case others liked the idea. Personally I prefer the visual que, but that's just me. I didn't know what icon to use so I just set them to display as 'btn'. They are ugly.

Just a nit, but we use the shouldXX naming for settings, not for regular predicates.

Gotchya, would just renaming to the same name that sets it be appropriate? So it could be: isMobileBrowser

Can you elaborate on why didn't you build on my comment from the initial issue (#586 (comment))?...I also prepared a Draft PR for you. Why didn't you build on that one?

I messed that one up. It didn't all click where I went wrong until just recently. I had just done my very first PR on another repo, where they branched off the develop branch, and much of what I've learned about using git came from reading where it is often told the master branch is reserved and shouldn't be worked on. Apparently the real world doesn't always line up with what we're taught it is, who would have guessed that? Anyways, sorry for complicating it and not finding clarity before just opening a new PR. There is one more lesson for me.

I'll get it all straightened. I appreciate your understanding and patience, it makes this new experience easier to figure out.

[munen]

No worries, if this is your second PR in git in total, then you're already doing an awesome job. Just keep chugging and learning :+1:

[markamaze]

Haven't forgotten about this, just have some family obligations. Will get back to it soon.

[munen]

This was closed accidentally.