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

Switch to 'markdown-it-task-checkbox' for rendering of task lists #3898

Merged
merged 3 commits into from
Jul 5, 2022
Merged

Conversation

q-wertz
Copy link
Contributor

@q-wertz q-wertz commented Jul 3, 2022

Signed-off-by: q-wertz clemens.sonnleitner@web.de

Summary

As suggested by @https://github.com/hrk I gave it a shot and tried to use the library markdown-it-task-checkbox instead of markdown-it-task-lists.

  • Not sure if some of the features that were added to markdown-it-task-lists are needed. I tried to get the same classnames for ulClass and liClass but e.g. the enabled class to the <li> elements (which has been introduced with v2.0.0 of markdown-it-task-lists) cannot be set (but did not find any references in the Deck app which seem to require this).
  • Also not sure whether PR Fix for issue #3637 #3833 has any side effects on this.

TODO

  • Double check code validity (I'm neither an experienced web developer nor do I have deep insights in Deck or Nextcloud)
  • Test

Checklist

  • Code is properly formatted
  • Sign-off message is added to all commits
  • Tests (unit, integration, api and/or acceptance) are included -> Don't think additional specific tests are required
  • Documentation (manuals or wiki) has been updated or is not required

Signed-off-by: q-wertz <clemens.sonnleitner@web.de>
@juliushaertl
Copy link
Member

Tested and works and the library also seem sane to use, thanks a lot for this 👍

@juliushaertl
Copy link
Member

Can you maybe also push the package-lock.json updates after running npm install --save markdown-it-task-checkbox While I could rebase through the web UI it seems I cannot push from my local machine to your branch.

@q-wertz
Copy link
Contributor Author

q-wertz commented Jul 5, 2022

👍

There were quite some warnings and the package-lock.json changed a lot (due to the rebasing?)...

$ npm install --save markdown-it-task-checkbox
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: 'deck@1.8.0-beta.0',
npm WARN EBADENGINE   required: { node: '^14.0.0', npm: '^7.0.0' },
npm WARN EBADENGINE   current: { node: 'v18.3.0', npm: '8.6.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@nextcloud/axios@1.10.0',
npm WARN EBADENGINE   required: { node: '^14', npm: '^7' },
npm WARN EBADENGINE   current: { node: 'v18.3.0', npm: '8.6.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@nextcloud/eslint-config@8.0.0',
npm WARN EBADENGINE   required: { node: '^14.0.0', npm: '^7.0.0' },
npm WARN EBADENGINE   current: { node: 'v18.3.0', npm: '8.6.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@nextcloud/stylelint-config@2.1.2',
npm WARN EBADENGINE   required: { node: '^14.0.0', npm: '^7.0.0' },
npm WARN EBADENGINE   current: { node: 'v18.3.0', npm: '8.6.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@nextcloud/vue@5.3.1',
npm WARN EBADENGINE   required: { node: '^14.0.0', npm: '^7.0.0' },
npm WARN EBADENGINE   current: { node: 'v18.3.0', npm: '8.6.0' }
npm WARN EBADENGINE }
npm WARN EBADENGINE Unsupported engine {
npm WARN EBADENGINE   package: '@nextcloud/webpack-vue-config@5.1.0',
npm WARN EBADENGINE   required: { node: '^14.0.0', npm: '^7.0.0' },
npm WARN EBADENGINE   current: { node: 'v18.3.0', npm: '8.6.0' }
npm WARN EBADENGINE }
npm WARN deprecated querystring@0.2.0: The querystring API is considered Legacy. new code should use the URLSearchParams API instead.

added 1609 packages, and audited 1610 packages in 47s

208 packages are looking for funding
  run `npm fund` for details

13 vulnerabilities (3 moderate, 10 high)

To address issues that do not require attention, run:
  npm audit fix

To address all issues possible (including breaking changes), run:
  npm audit fix --force

Some issues need review, and may require choosing
a different dependency.

Run `npm audit` for details.

Signed-off-by: q-wertz <clemens.sonnleitner@web.de>
@juliushaertl juliushaertl merged commit 0686575 into nextcloud:master Jul 5, 2022
@juliushaertl
Copy link
Member

/backport to stable24

@backportbot-nextcloud
Copy link

The backport to stable24 failed. Please do this backport manually.

@q-wertz
Copy link
Contributor Author

q-wertz commented Jul 13, 2022

Is this also backported to NC 23?

@q-wertz
Copy link
Contributor Author

q-wertz commented Sep 20, 2022

Is there any information when this will be in any stable version? 😅

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

Successfully merging this pull request may close these issues.

Checklists break (markdown) styled text in description field
2 participants