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

Clicking the initial "Write your story" prompt marks a new post as dirty #1501

Closed
nylen opened this issue Jun 27, 2017 · 17 comments
Closed

Clicking the initial "Write your story" prompt marks a new post as dirty #1501

nylen opened this issue Jun 27, 2017 · 17 comments
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Milestone

Comments

@nylen
Copy link
Member

nylen commented Jun 27, 2017

The following state was reached by simply clicking the "Write your story" / "Continue writing" placeholder multiple times:

This seems undesirable. The "Continue writing" prompt should only appear if the last block in the post is not empty (and perhaps if it is not a text block). This part was fixed in #1553.

Also, the first click on the initial "Write your story..." placeholder creates a text block, which marks a new post as dirty (note appearance of "Save" link in top left):

2017-06-27t15 56 07 0200

If possible it would be nice to avoid marking the post dirty in this situation.

@nylen nylen added [Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended labels Jun 27, 2017
@youknowriad
Copy link
Contributor

How would you check that the last block is not empty? I mean outside of the block context, we can't know.

@nylen
Copy link
Member Author

nylen commented Jun 27, 2017

It's a good question, I didn't see a quick way to do it. The current behavior is honestly pretty annoying though.

@karmatosed
Copy link
Member

A user reported this issue in test:

"I unintentionally created 3 extra blank blocks before properly adding the image blog"

@nylen
Copy link
Member Author

nylen commented Jun 28, 2017

Only the second part of this issue is still valid now.

Clicking the initial "Write your story..." placeholder creates a text block, which marks a new post as dirty (note appearance of "Save" link in top left):

2017-06-27t15 56 07 0200

If possible it would be nice to avoid marking the post dirty in this situation.

@nylen nylen changed the title Revise behavior of "Continue writing" prompt Clicking the initial "Write your story" prompt marks a new post as dirty Jun 28, 2017
@nitrajka
Copy link
Contributor

nitrajka commented Aug 25, 2017

Hi,

what if this is the intended behavior?
screen shot 2017-08-25 at 09 49 10

User comes and clicks on 'Write your story'. The new and first block is appended to the state and so saved as a content. This isNew post which becomes now isSaveable becomes also isDirty. The post converts from' auto-draft' to 'draft' (isNew changes to false) and the post is saved to the state and the database. Now, when the post isSaveable, the AUTOSAVE function is being called every 10 seconds while the post isDirty.
Although a user didn't write a letter. But he has already inserted some content. Saving the first block to the state triggers all of the changes.
Is this behavior really annoying? I don't think so. After all, a user has just inserted a content which needs to be saved. Makes sense?

I debugged this for a while. Maybe the last point is worthy of thinking about, although I can't imagine how much time would it take.

  1. We could check whether the block has a content and firstly then save it to the state. But this wouldn't work because the block appears only then if it has already been saved to the state/database.
  2. We could fake the block somehow. So it appears as a real block, but it hasn't been saved to the state yet. Quite complicated and inelegant.
  3. We could have one default block which is already inserted in the state and user can not delete it. Like the title. Makes sense, doesn't work. What if a user wants to have an image in the first block and not a text?
  4. We could have one default block. For example Text. A user creates an image block and deletes the text block. So he changed the default block type, but not the feature - one default block. But this also wouldn't work, because a user had to create one more block (except for the default) and so he has already created some content which makes the post saveable.
  5. We could save the very first block in redux state without saving it to the database. And when the post changes from 'auto-draft' to 'draft' then will be made a post request to the database.

I would self-assign the issue, and really would like to work on it, but all of the solutions are a bit creepy. And I don't think there is some elegant solution for this 'maybe' issue. It's the saving flow, which would need a change. Quite time-demanding for such a small annoying issue. What do you think?

@youknowriad
Copy link
Contributor

@nitrajka What if we take a similar approach to Calypso (see https://github.com/Automattic/wp-calypso/blob/a70587157b33783ba4d5e0e4a1e136680d459d51/client/state/selectors/edited-post-has-content.js#L19-L21).

Testing the HTML instead of testing the blocks and excluding empty paragraphs and comments (regexp)

@nitrajka
Copy link
Contributor

nitrajka commented Aug 25, 2017

@youknowriad Yes, I thought about it but didn't think it is a correct solution for our case.
Do you mean, to test whether the content of every single block is empty or just the very first one? I think, we shouldn't test every empty block. What if it is intended to be the empty line between two paragraphs? You know, user inserted it purposely. We delete/don't save it just because it's empty?

@youknowriad
Copy link
Contributor

I think we should test the entire post, if it has content or not, and assume that moving the post from var content = ""; to var content="<!--><p></p><!-->"; keeps the dirty state to false (for non-dirty posts)

@nitrajka
Copy link
Contributor

@youknowriad makes sense. Thank you. Let's try it.

@aduth
Copy link
Member

aduth commented Aug 28, 2017

Worth noting that in Calypso's case, it is specifically handling the fact that TinyMCE content will pad itself with paragraph and/or "bogus" br nodes even when empty. This does not apply in Gutenberg, and it's not accurate to say that a post is empty because a block doesn't have content, specifically in the case of dynamic (server-rendered) blocks.

@youknowriad
Copy link
Contributor

This does not apply in Gutenberg, and it's not accurate to say that a post is empty because a block doesn't have content, specifically in the case of dynamic (server-rendered) blocks.

That's a good point, maybe we could explicitly do so for text blocks only.

@aduth
Copy link
Member

aduth commented Aug 28, 2017

Honestly, this feels like "Working as Intended" to me. Even if a block doesn't have readable content, the post has a block. If we consider emptiness as presence of any blocks, it follows that the post is not empty, even if the paragraph has no content.

We could get into weeds of what it means for a block to be empty, but this seems like it'd eventually lead to either blocks implementing their own isEmpty logic or targeting specific blocks, neither of which seems convenient or stable.

@mtias
Copy link
Member

mtias commented Aug 31, 2017

A block like paragraph could also have a "placeholder" set up.

I still think it would be nice to not save so many new drafts just by clicking and clicking out.

@nitrajka
Copy link
Contributor

nitrajka commented Sep 5, 2017

@aduth exactly. I've also written about it in my comment above. I also think it is intended behavior.
Does it make sense to implement isEmpty logic for the block? No. What if a user wants to have an empty line between two paragraphs. Do we delete/not save it just because it's empty? I agree.

@mtias Do you think, that the "placeholder" set up could/should solve the problem with saving drafts just by clicking (out)?
Wouldn't be possible to implement it like the title? (If you click and click out of a title, the post won't save as a draft.)

@mtias
Copy link
Member

mtias commented Sep 5, 2017

@nitrajka if there are multiple placeholders set up (a heading, a paragraph, an image), those need (at the moment) to exist in the post content.

@nitrajka
Copy link
Contributor

nitrajka commented Sep 5, 2017

@mtias Okay. I just wasn't sure how you meant it. So if I understand it well, the newly created post will already have some content, right? Will be such post considered as auto-draft or draft?

@nitrajka nitrajka removed their assignment Sep 21, 2017
@karmatosed karmatosed added this to the Merge Proposal milestone Jan 25, 2018
@youknowriad
Copy link
Contributor

Per last discussions cc @aduth , it seems that we want to consider A post with an empty paragraph as not empty and thus dirty. We do have the technical pieces to achieve this ticket pretty easily (with isDefaultUnmodifiedBlock helper but let's close this ticket for now. We could reopen and fix if the consensus changes.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Feature] Blocks Overall functionality of blocks [Type] Bug An existing feature does not function as intended
Projects
None yet
6 participants