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.

Sign up for GitHub

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

Jump to bottom

Fix for history that's not empty on a fresh start of the editor #892

Merged
merged 6 commits into from
Apr 23, 2019
Merged

Fix for history that's not empty on a fresh start of the editor #892

merged 6 commits into from
Apr 23, 2019

Conversation

daniloercoli
Copy link
Contributor

@daniloercoli daniloercoli commented Apr 18, 2019 •
edited
Loading

This PR fixes #887

All the changes are in the GB side companion PR WordPress/gutenberg#15055

Test 1

  • Start the demo app
  • Tap on a RichText block
  • Check the Undo/Redo button are not available

Test 2

  • Start the demo app
  • Tap on a RichText block
  • Check the Undo/Redo button are not available
  • Go to the end of the block
  • Tap enter to create a new para
  • Tap Undo twice* and the new block should be gone
  • => there is another problem with history that I will tackle in another PR. Shortly, Aztec can modify the HTML tags order, so pressing enter on a block does result in sending back to the store an HTML content that could be slightly different from the one set at the beginning, resulting in a new history entry**

@daniloercoli daniloercoli added [Type] Bug Something isn't working Writing Flow labels Apr 18, 2019
@daniloercoli daniloercoli added this to the v1.4 milestone Apr 18, 2019
@daniloercoli daniloercoli mentioned this pull request Apr 18, 2019
Merged
@mkevins
Copy link
Contributor

mkevins commented Apr 19, 2019

I tested this, and it does fix the issue, as described.

I have some additional findings documented in this comment: #887 (comment)

One other thing I did notice is that when I created a new paragraph block from the problematic italicized block, two taps on undo was sufficient to clear the new block, but a third tap was necessary to fully clear the undo history and see the undo button disabled.

@daniloercoli
Copy link
Contributor Author

One other thing I did notice is that when I created a new paragraph block from the problematic italicized block, two taps on undo was sufficient to clear the new block, but a third tap was necessary to fully clear the undo history and see the undo button disabled.

Yes, this is the issue I mentioned in the description (sorry if i was not clear, and still to open a new issue for it).
Basically when you tap enter on the first block the content of the block is retrieved from Aztec and stored in the store, and a history point made for it. Unfortunately Aztec does change the HTML given in input, even if you don't actually change the block, so the JS side notices the change and create the undo point.

Will open a new issue shortly with details.

@daniloercoli
Merge branch 'develop' of https://github.com/wordpress-mobile/gutenbe…
baa91a4 
…rg-mobile into issue/887-History-stack-is-not-empty-on-a-fresh-start-of-the-editor

* 'develop' of https://github.com/wordpress-mobile/gutenberg-mobile:
  resolve syntax error in config
  install jest-junit locally
  use serverconfig auth value for hash
  log jobURL at end of successful job
  add environment for ci checks
  fix directory path for reports
  fix directory path for reports
  install jest reporter for ci checks script as well
  include test reporter for UI tests
@daniloercoli daniloercoli mentioned this pull request Apr 19, 2019
Open
@daniloercoli
Upgrade GB ref
d3b1eea
@daniloercoli
Merge branch 'develop' of https://github.com/wordpress-mobile/gutenbe…
0e2bb58 
…rg-mobile into issue/887-History-stack-is-not-empty-on-a-fresh-start-of-the-editor

* 'develop' of https://github.com/wordpress-mobile/gutenberg-mobile:
  Improving accessibility on Post Title (#916)
  Point to GB master without the list block suppression
  Update JS bundles
  Update Gutenberg hash with lint error fixes
  Update JS bundles
  Suppress the List block on Android
  Updated JS bundles with title focus fix
  Update JS bundles for 1.3.0
  Update version to 1.3.0
  Update AztecEditor-iOS to 1.6.0
  Update gutenberg ref to latest master
  Update gutenberg ref
  Update gutenberg ref

# Conflicts:
#	gutenberg
@daniloercoli
Update GB ref
ac2c867
mkevins
mkevins approved these changes Apr 23, 2019
View reviewed changes
Copy link
Contributor

@mkevins mkevins left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tested this, and it is working well. LGTM!

@daniloercoli daniloercoli merged commit c71e081 into develop Apr 23, 2019
@daniloercoli daniloercoli deleted the issue/887-History-stack-is-not-empty-on-a-fresh-start-of-the-editor branch April 23, 2019 10:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mkevins mkevins mkevins approved these changes

@Tug Tug Awaiting requested review from Tug

@marecar3 marecar3 Awaiting requested review from marecar3

Assignees
No one assigned
Labels
[Type] Bug Something isn't working Writing Flow
Projects
None yet
Milestone
v1.4
Development

Successfully merging this pull request may close these issues.

History stack is not empty (and broke) on a fresh start of the editor, and on new RichText block
2 participants
@daniloercoli @mkevins