Skip to content
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

Place TODO keyword of previous header in the new header #164

Merged
merged 4 commits into from
Dec 28, 2019

Conversation

schoettl
Copy link
Collaborator

This should close #20

@munen When editing the title of a header (pressing the title edit icon), shouldn't we place the curser at the end of the line by default? At least for new headers (with a TODO keyword) this would make very much sense. And for existing headers with much text it wouldn't hurt, what do you think?

@schoettl
Copy link
Collaborator Author

Setting the curser position in the text input works on Arch Chromium and Firefox and Android Chrome.

Can you test on iPhone before merge?

@schoettl schoettl changed the title [WIP] Place TODO keyword of previous header in the new header Place TODO keyword of previous header in the new header Dec 28, 2019
@munen munen force-pushed the feature/20/add-header-with-todo branch from 09d6fff to e36ab63 Compare December 28, 2019 14:20
@munen
Copy link
Collaborator

munen commented Dec 28, 2019

I really like the implementation of "Place TODO keyword of previous header in the new header". Nice that you put your first code into a reducer(;

I'll write an acceptance test for it and merge.

As for:

When editing the title of a header (pressing the title edit icon), shouldn't we place the curser at the end of the line by default?

I understand that you like this. However, it would be breaking UX change. Plus, for me personally, I don't think it would be a good default. My headlines are not all short, so when having the cursor at the end, I might not be able to read the beginning of the headline.

Having said this, I really do understand that you like this. I removed your commit from this PR (as it is a separate feature) and opened a new PR for it. We can totally add the feature, but my proposal is that we introduce a setting for the behavior with the default set to false. Would that work for you?

@munen munen merged commit 7f3f8a8 into 200ok-ch:master Dec 28, 2019
@munen
Copy link
Collaborator

munen commented Dec 28, 2019

@schoettl Great new feature, thanks for implementing it! 🙏

I've written an acceptance test and updated the changelog with attribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants