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

fix(markdown-widget): apply list item style on each block in a selection #5676

Merged

Conversation

bytrangle
Copy link
Collaborator

@bytrangle bytrangle commented Aug 3, 2021

fixes #5654

Summary
When selecting multiple paragraph blocks and click List button, the current behaviour is to turn them into one list block which contains one list item, no matter how many paragraph blocks are selected.

This PR changes that behavior to make Netlify CMS editor more in line with the convention in most rich text editors. Each paragraph block in the selection will be wrapped with a respective list item. Then all the list items will be wrapped with one unifying list block.

HTML before clicking List button:

<p>Item 1</p>
<p>Item 2</p>
<p>Item 3</p>

HTML after clicking List button:

<ul>
  <li>
      <p>Item 1</p>
  </li>
  <li>
      <p>Item 2</p>
  </li>
  <li>
      <p>Item 3</p>
  </li>
</ul>

Test plan
TODO:

  • Write a test to check if all bottom-most blocks in a selection are turned into list items when clicking List button
  • Write a test to check if all list items in a selection are turned back into paragraph blocks when clicking List button

Checklist

Please add a x inside each checkbox:

  • I have read the contribution guidelines.
  • Code is formatted via running yarn format.
  • Tests are passing via running yarn test.
  • The status checks are successful (continuous integration). Those can be seen below.

@bytrangle bytrangle requested a review from a team August 3, 2021 05:33
@erezrokah erezrokah added the type: bug code to address defects in shipped code label Aug 3, 2021
@bytrangle
Copy link
Collaborator Author

Hi @erezrokah,
I recently changed my OS and had to set up Yarn from scratch. After running yarn bootstrap, I ended up with more than 2000 pending changes including changes to the following Yarn-related files:

  • addition of a folder name .yarn. There's a topic on Yarn saying that it should be committed, but really?
  • yarn.lock
  • yarnrc.yml
  • netlify-cms-proxy-server/package.json with the addition of this line: `"bin": "./dist/index.js".

What files should I commit?

@bytrangle bytrangle changed the title Fix/list style on multi block selection fix(markdown-widget): apply list item style on each block in a selection Aug 4, 2021
@erezrokah
Copy link
Contributor

Hi @erezrokah,
I recently changed my OS and had to set up Yarn from scratch. After running yarn bootstrap, I ended up with more than 2000 pending changes including changes to the following Yarn-related files:

  • addition of a folder name .yarn. There's a topic on Yarn saying that it should be committed, but really?
  • yarn.lock
  • yarnrc.yml
  • netlify-cms-proxy-server/package.json with the addition of this line: `"bin": "./dist/index.js".

What files should I commit?

Which version of yarn are you using? Maybe this is related to yarn v1 -> v2 changes?

@erezrokah erezrokah force-pushed the fix/list-style-on-multi-block-selection branch from 7ae160b to 8aafcd3 Compare August 4, 2021 13:51
@bytrangle
Copy link
Collaborator Author

It is 1.23.0, after I had run yarn set version classic because lerna bootstrap doesn't work on Yarn v2.

@erezrokah
Copy link
Contributor

erezrokah commented Aug 4, 2021

It is 1.23.0, after I had run yarn set version classic because lerna bootstrap doesn't work on Yarn v2.

So I'm not sure. I'm using 1.22.5 and our CI uses 1.22.11, so maybe try aligning your version to one of those. If you'd like you can commit the files to a separate branch and I'll try to see what happens when I run yarn bootstrap on that branch.

On another note, I've added two commits, one a refactoring and one a fix to set the correct list type.

Copy link
Contributor

@erezrokah erezrokah left a comment

Choose a reason for hiding this comment

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

This looks 🚀 Those new tests are 💯

@bytrangle
Copy link
Collaborator Author

Thank you for all those correction. I learned a lot ✨.

@erezrokah erezrokah merged commit 04e5305 into decaporg:master Aug 4, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: bug code to address defects in shipped code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Editor: Selecting X paragraphs and clicking List button creates a list which contains only one item.
2 participants