search

pkp / pkp-lib

The library used by PKP's applications OJS, OMP and OPS, open source software for scholarly publishing.
https://pkp.sfu.ca
GNU General Public License v3.0
297 stars 443 forks source link

Section word count field is inoperative #2571

Closed asmecher closed 6 years ago

asmecher commented 7 years ago

When you edit a Section, it's possible to provide a maximum word count.

The field isn't used or displayed anywhere.

asmecher commented 7 years ago

@stranack, preferences?

ajnyga commented 7 years ago

I will put my spoon in this soup as well (sorry). I suggest:

Alinex29 commented 7 years ago

I agree with @ajnyga's suggestions. It would be truly helpful if we could use the word count field as a filter.

asmecher commented 6 years ago

Deferring from OJS 3.1.

bozana commented 6 years ago

PRs: tinymce master: https://github.com/pkp/tinymce/pull/25 pkp-lib master: https://github.com/pkp/pkp-lib/pull/3061 ojs master: https://github.com/pkp/ojs/pull/1694 quickSubmit master: https://github.com/pkp/quickSubmit/pull/17

bozana commented 6 years ago

@NateWr, could you review the PRs? THANKS!!!

NateWr commented 6 years ago

👍

  1. It only seems to check the primary locale. If I include a short abstract in the primary locale and a too-long abstract in a secondary locale, the form submits without an error.

  2. It'd be great if we can show the current abstract length as they type. It looks like there's a wordcount plugin for tinymce. Maybe that will do the trick? If not, perhaps we can listen for changes in the tinymce content and update our own counter?

bozana commented 6 years ago

Thanks a lot @NateWr! I'll take a look...

bozana commented 6 years ago

@NateWr, I considered all suggestions (except providing the wordCount parameter in the form validation message, because this is not possible at the moment). Could you take a look? I tried to made the wordcount plugin generic for textareas, but I could also only consider the id^='abstract-'... if so wished... but it's maybe better this way, because also abstract may not have limit... I believe it is better to use the original tinymce plugin, because it already displays it correctly for our multilingual fields, so we do not have to think where and how to display it...

NateWr commented 6 years ago

Cool! Nice to be able to tack on the word count to any rich text area.

I was hoping the word count plugin would support a max-length setting so that it turned red or something when exceeded, but I think this will do the trick for now.

I'm running into an issue with the word count disappearing when the field is reloaded as part of an error.

image

To reproduce:

  1. Go to new submission.
  2. Add an abstract that's too long.
  3. Save.

When the field is reloaded, the word count is missing, and the error message has taken over the word count limit message, so there's no indication of what the word limit is.

This doesn't happen on the metadata form -- the word count doesn't disappear and the form error message doesn't take over the original field description.

bozana commented 6 years ago

Great catch @NateWr!!! I had to add the wordCount as a form filed and to read it in readInputData. Two new commits came. It should work now :-)

But, I figured out something else: the problem was not visible in the submission metadata form, because this form is always reloaded anew, s. https://github.com/pkp/pkp-lib/blob/master/controllers/tab/publicationEntry/PublicationEntryTabHandler.inc.php#L168-L174. Thus all form changes (not only for the abstract field) are then lost, if an error occurs and the form is reloaded :-P This mechanism is often used, I believe, and maybe to tackle it once later, considering all those similar cases, if needed... ?

NateWr commented 6 years ago

👍 Works well now. I left just a couple final comments on the PRs.

Yes, one of my big goals with Vue.js is to refactor our form handling to address some of these issues.

NateWr commented 6 years ago

Oh, I forgot to mention another issue I ran into: I could exceed the word count by a little bit. I found I could get the tinymce word count to read 120, and successfully submit the abstract, even though I had set the word limit to 100. Probably some discrepancy in how the count is being calculated.

bozana commented 6 years ago

Ah, yes, I've forgotten to check that... :-\ I'll take a look. Thanks a lot!!!

bozana commented 6 years ago

@NateWr, I introduced a new PKP wordcount tinymce plugin (there is a new PR for tinymce repository above) -- to be able to use the same algorithm for word counting. Now the wordcount also turns red, if the limit is reached. The only thing I did not managed is to position the word count in the right corner :-P I also removed the wordCount hidden field, changed the validation message and adopt the validation functions to be the same. Could you please another look? THANKS!!!

NateWr commented 6 years ago

@bozana Really nice! The color change on the word count makes a big difference. I noticed a few lingering issues:

  1. When the form is submitted with an abstract that's too long, it doesn't save any of the changes. This could be really annoying if you typed in a whole abstract. Maybe it makes sense to try and save the first 100 words? I imagine that will be difficult with the existing validation system.

  2. The error message still appeared behind the modal for me.

  3. I didn't see the word count when making a submission. Is it missing there?

I may have an idea to address 1 and 2. If you leave a required field, like the title, empty, the form submission JS blocks the form from being submitted. Maybe we can do the same if a field is over the word count limit? That way we wouldn't need to display an error message after saving, and the user would get to keep their abstract if they tried to submit it when it was over the limit. What do you think?

bozana commented 6 years ago

Yes, those two are those strange ones, that would need some special attention in general... :-) But your idea sounds great -- I will try to do that... And I think I have forgotten the submit form i.e. number 3 above... :-P Will do... Thanks a lot!!!

bozana commented 6 years ago

@NateWr, guess what! I managed to solve those first two points/issues correctly/actually :tada: :fireworks: :dancer: S. this commit: https://github.com/pkp/pkp-lib/pull/3061/commits/a70b4db1499bfce595df214d23869237126b26e4. Thus, also the same problem mentioned here https://github.com/pkp/pkp-lib/issues/3157 is solved :clap: :raised_hands: I also considered the section word count in the submit step 3 form. Thus, I think now everything is there, so could you please take a look at this one more, hopefully final time? THANKS!!!

NateWr commented 6 years ago

:tada: Woohoo! I tested it and did a nother pass on the code and it all looks great. Merge whenever you're ready.