Bump publiclab-editor from 2.0.6 to 2.1.0

[dependabot-preview]

Bumps publiclab-editor from 2.0.6 to 2.1.0.

Release notes

Sourced from publiclab-editor's releases.

Editor revamp 2020 first release

Quoting @shreyaa-sharmaa in publiclab/PublicLab.Editor#555

Great work, team!

Over a month into GSoc and a month and half into Outreachy, we are ready to release a new version of PLE ❤️ These are some of the changes introduced:

Changes for users

1. Remove File Button Added: #472 Now the image added in the main image module could be removed with just a click.
2. Fix: Disabling of Markdown buttton on using shortcut keys #465 Earlier pressing CTRL+M disabled the Markdown button. Now, It has been fixed.
3. A better UI with a toolbar that hovers only over the textarea (#522) and improved Choose file button (#462)

Additions

1. Jest-puppeteer for UI testing by @shreyaa-sharmaa in #532 #541
2. User workflow for documenting bugs by @Shulammite-Aso in #530
3. Issue template chooser by @Shulammite-Aso in #474

Modifications/ Improvements

1. Remove file button added by @NitinBhasneria in #472
2. UI refinements: Stickiness of toolbar by @Shulammite-Aso in #522
3. UI refinements: Choose file button UI improved by @NitinBhasneria in #462
4. Nodejs version updated to 10 and 12 by @shreyaa-sharmaa in #554
5. Fix: Disabling of Markdown buttton on using shortcut keys by @keshav234156 in #465

Commits

• See full diff in compare view

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.

---

Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

• @dependabot rebase will rebase this PR
• @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
• @dependabot merge will merge this PR after your CI passes on it
• @dependabot squash and merge will squash and merge this PR after your CI passes on it
• @dependabot cancel merge will cancel a previously requested merge and block automerging
• @dependabot reopen will reopen this PR if it is closed
• @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
• @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
• @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)
• @dependabot badge me will comment on this PR with code to add a "Dependabot enabled" badge to your readme

Additionally, you can set the following in the .dependabot/config.yml file in this repo:

• Update frequency
• Automerge options (never/patch/minor, and dev/runtime dependencies)
• Out-of-range updates (receive only lockfile updates, if desired)
• Security updates (receive only security updates, if desired)

[codecov]

Codecov Report

Merging #8135 into main will decrease coverage by 0.12%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##             main    #8135      +/-   ##
==========================================
- Coverage   82.17%   82.05%   -0.13%     
==========================================
  Files         100      100              
  Lines        5772     5772              
==========================================
- Hits         4743     4736       -7     
- Misses       1029     1036       +7     

Impacted Files	Coverage Δ
app/services/execute_search.rb	88.88% <0.00%> (-5.56%)	⬇️
app/api/srch/search.rb	66.24% <0.00%> (-3.83%)	⬇️

[jywarren]

Ok folks! Let's see what happened. It's possible we broke one of the assumptions the editor makes for the integration by plots2. @Shreyaa-sharmaa @Shulammite-Aso @keshav234156 @NitinBhasneria publiclab/PublicLab.Editor#555

[jywarren]

Yes, check the system tests here!

Minitest::UnexpectedError:         Selenium::WebDriver::Error::JavascriptError: javascript error: Cannot set property 'files' of null
          (Session info: headless chrome=73.0.3683.86)
          (Driver info: chromedriver=73.0.3683.68 (47787ec04b6e38e22703e856e101e840b65afe72),platform=Linux 4.15.0-1028-gcp x86_64)
            test/application_system_test_case.rb:20:in `drop_in_dropzone'
            test/system/rich_text_editor_test.rb:36:in `block in 

Perhaps we have to issue a patch level update to fix this before we can merge here?

[jywarren]

Ah, it actually looks like we have a few errors, but small ones!

https://travis-ci.com/github/publiclab/plots2/jobs/360140901#L3166 is the one above, to do with the files attribute of something.

Then we have a couple others, at https://travis-ci.com/github/publiclab/plots2/jobs/360140901#L3255-L3276

Minitest::UnexpectedError:         Capybara::ElementNotFound: Unable to find css ".wk-wysiwyg"
            test/system/post_test.rb:23:in `block in <class:PostTest>'

and

Minitest::UnexpectedError:         Capybara::ElementNotFound: Unable to find css ".wk-wysiwyg"
            test/system/post_question_test.rb:77:in `block in <class:QuestionTest>'
[Screenshot]: tmp/screenshots/failures_test_posting_a_question.png

and

Minitest::UnexpectedError:         Capybara::ElementNotFound: Unable to find css ".wk-wysiwyg"
            test/system/post_question_test.rb:59:in `block in <class:QuestionTest>'

These last 3 actually all look like separate instances of the same. Something has happened to make .wk-wysiwyg not load! Let's look at the commits that went into this release and try to trace back what may have happened.

No worries, this is a big release and I'm sure we'll iron it out! 🎉

[NitinBhasneria]

I have checked this error manually in my system and all the error are from the class .wk-wysiwyg, @keshav234156 @Shulammite-Aso @Shreyaa-sharmaa does anyone one of you have changed this class anywhere. Please have a look as this error is occurring because in the required files this class container does not exist.

[cypherean]

Checked my PRs and none of them had any change to this class. Any lead on the first error mentioned, the one about files? I'm trying to dig into this too. But no success so far.

[cypherean]

Went through most of the PRs that have merged in the past 2 months but none of them had any changes related to this class. @NitinBhasneria can you check this PR publiclab/PublicLab.Editor#437 with the first error. This is the only one that connects to the classes/parts included in the first error though I couldn't figure out how or why it would cause this. Please also have a look at any other PR you've made for changes in the file drag and drop.

[NitinBhasneria]

@jywarren @sagarpreet-chadha while releasing the changes of publiclab.editor in plots2 don't we have to update the https://github.com/publiclab/plots2/blob/main/app/views/editor/rich.html.erb file with https://github.com/publiclab/PublicLab.Editor/blob/main/examples/index.html as we have done many changes in index.html.
I am confused here can you clear my doubts. Thanks.

[sagarpreet-chadha]

Can you check if puppeteer is direct dependency in PL editor or not. Thanks 😄

[sagarpreet-chadha]

This should not come.
Have we not added puppeteer as dev dependency in package.json of PL editor?

[cypherean]

puppeteer is added as a dependency and package.json was updated.

[sagarpreet-chadha]

It should be dev dependency and not direct dependency.

[cypherean]

I'll update it and open a new PR.

[cypherean]

publiclab/PublicLab.Editor#565 here it is.

[jywarren]

Merged that, thank you!

[sagarpreet-chadha]

Hey Nitin,
If in the new PL editor version, there is change in functions that were called earlier in plots2 or the way we used to call those functions has changed or the function name is changed or any new feature is added --- then yes we need to change rich.html.erb in plots2.

[sagarpreet-chadha]

Also from the new version number, it seems that it is a minor change...so ideally there should be backward compatibility of code, so that it does not break anything if library gets auto upgraded due to use of ^ in package.json
See this https://bytearcher.com/articles/semver-explained-why-theres-a-caret-in-my-package-json/

I hope this clears your doubt! Thanks :)

[NitinBhasneria]

Hey Nitin,
If in the new PL editor version, there is change in functions that were called earlier in plots2 or the way we used to call those functions has changed or the function name is changed or any new feature is added --- then yes we need to change rich.html.erb in plots2.

Yea, I guess this may be the reason of the test fail then, @jywarren can you confirm. Thanks @sagarpreet-chadha

[jywarren]

OK, i agree with @sagarpreet-chadha that we should try to avoid requiring changes to downstream users like plots2 if we can, esp. without releasing a new major version (which would imply breaking changes). So, @NitinBhasneria or @Shreyaa-sharmaa could you highlight the exact changes to index.html which would have to be reflected in rich.html.erb here? If we can develop a way to use more flexible CSS perhaps we can avoid the change in requirements.

But if it's an important structural change, we can probably just add very clear guidance to our release notes indicating a breaking change. It's just something we'd like to avoid as a matter of principle unless we are really planning ahead with a collection of breaking changes and have decided it's worth releasing a major version v3.0.0. Make sense?

Thanks for all the careful research here! Let's prioritize this so we can get this merged and published! Thanks, all!

[sagarpreet-chadha]

Agreed! 👍 💯

[NitinBhasneria]

So, @NitinBhasneria or @Shreyaa-sharmaa could you highlight the exact changes to index.html which would have to be reflected in rich.html.erb here?

These are the points of difference:

1. Firstly, in rich.html.erb there is no selector module, while we have a selector module in index.html.
2. In index.html Title module in input the id and aria-label is removed which will make the error in test of plots2.
3. Main Image Module has the major changes in index.html, these changes should be their inrich.html.erb or we can't have the image fileName and remove the file button.
4. Minor indentation changes in Map Module.
5. Body module is having the pre-existing text in index.html which cant be seen in rich.html.erb.
6. Tag module is commented out in rich.html.erb, while not in index.html.

[sagarpreet-chadha]

Looks like there are major changes in the new version, we need to make these changes in this PR.
From next time we will make sure that the package version is updated correctly. No worries, thanks for the update 🎉 💯

[cypherean]

So, @NitinBhasneria or @Shreyaa-sharmaa could you highlight the exact changes to index.html which would have to be reflected in rich.html.erb here?

These are the points of difference:

1. Firstly, in rich.html.erb there is no selector module, while we have a selector module in index.html.
2. In index.html Title module in input the id and aria-label is removed which will make the error in test of plots2.
3. Main Image Module has the major changes in index.html, these changes should be their inrich.html.erb or we can't have the image fileName and remove the file button.
4. Minor indentation changes in Map Module.
5. Body module is having the pre-existing text in index.html which cant be seen in rich.html.erb.
6. Tag module is commented out in rich.html.erb, while not in index.html.

@NitinBhasneria can you try making these changes locally and check if it works? I've tried but I'm running into a lot of errors in different files. Meanwhile, I'll try resolving them. Thanks!

[NitinBhasneria]

@NitinBhasneria can you try making these changes locally and check if it works? I've tried but I'm running into a lot of errors in different files. Meanwhile, I'll try resolving them. Thanks!

Yea, when making the changes of index.html the ".wk-wysiwyg" error will be removed for sure( I have tried this locally too). While the title module of the index.html will make the error because

plots2/app/views/editor/rich.html.erb

Line 70 in 92834ea

id="title-input"

this line is removed in index.html.
Other than this everything is fine.

[jywarren]

Hi @NitinBhasneria that's great, can you open a PR to prove this out? And, ideally, we should try to fix this in the Editor, not in plots2, because it's the Editor that introduced a breaking change to a convention that plots2 relied upon, you know?

But as this is really getting delayed, I would be happy to accept a PR for either or both solutions. Thank you!

[sagarpreet-chadha]

Yes @NitinBhasneria , let's prioritize this first 💯
Kindly make the patches in this PR first, so that we are ready to deploy this 🎉
Thanks!

[sagarpreet-chadha]

In PL editor, I don't think we can remove a published version from NPM. In the repository itself we should mark the latest release as UNSTABLE or have BREAKING CHANGES.

[NitinBhasneria]

hey all, I have tried this PR locally on my system.
the CSS error will be corrected when we replace the mainImageModule and body module in plots2/app/views/editor/rich.html.erb from the PublicLab.Editor/examples/index.html.
Error is occurring because the id 'thumbnail-imgdoes not exist in existingplots2/app/views/editor/rich.html.erbin the main Image module. And so we are getting the errorCannot read property 'addEventListener' of null` due to which toolbar disappears and the CSS ".wk-wysiwyg" does not exist.

Now, of course, the way to do it is to replace the main image module in plots2/app/views/editor/rich.html.erb from the PublicLab.Editor/examples/index.html. The error will be removed for sure(I have checked it).
But, again the questions/new page does not contain the image module due to which again the error occurs of `Cannot read property 'addEventListener'.

So, there are two ways to do it now:

1. We can ad the image module input line in questions/new and make its display: none.
2. The other way to do this is to make the little change in PublicLab.Editor/src/modules/PublicLab.TitleModule.Related.js and again release a new version of PublicLab Editor.
   These are the only two ways.

@jywarren @sagarpreet-chadha have a look and decide the procedure.

[jywarren]

I think this is a perfect example of a good reason to patch upstream, in the Editor. The new additional requirement we accidentally introduced of a mainImageModule shouldn't break use cases which don't have that. So, let's:

make the little change in PublicLab.Editor/src/modules/PublicLab.TitleModule.Related.js and again release a new version of PublicLab Editor.

So we first check for the thumbnail-img element before trying to do anything to it. Then we issue that as a patch release v2.1.1 of the Editor, and close this PR in favor of the new one Dependabot will open, which shouldn't cause any errors 🤞

@NitinBhasneria can you add that line?

It's just wrapping this in a conditional to check it exists:

https://github.com/publiclab/PublicLab.Editor/blob/f9b0d292ce5b35ee3f6a2bf8b72e8b9f4ce364a8/src/modules/PublicLab.MainImageModule.js#L147-L150

Thank you! Great investigating! Let's aim to avoid introducing new breaking requirements in the next release, but this is a great exercise for dealing with downstream expectations!

Thanks, all! 🎉

[NitinBhasneria]

@NitinBhasneria can you add that line?

Yea, I will make a PR for the same. Thanks

[sagarpreet-chadha]

Hey @NitinBhasneria , a small change in that PR, then let's merge that and @jywarren will release the patch version (maybe on Tuesday). Thanks, great work 🎉 💯

[jywarren]

OK, noting the issue was solved by @NitinBhasneria here: publiclab/PublicLab.Editor#587

Bumping editor to v2.1.1 here: publiclab/PublicLab.Editor#588 by @sagarpreet-chadha - excellent

We should see Dependabot open a new PR once that's published!

[jywarren]

cc @publiclab/editor team! We should link the new PR to here to maintain continuity, but this one will close!

[jywarren]

Publishing v2.1.1 now!

[dependabot-preview]

Superseded by #8285.