Moodle
  1. Moodle
  2. MDL-37565

Better looking tinymce editor: toggle toolbars between 1 and 3 lines

    Details

    • Testing Instructions:
      Hide

      Test 1

      1. Start with an unpatched clean install
      2. Apply the patch and upgrade
      3. Ensure the toolbar config has been changed from the original default

      Test 2

      1. Open a page with 1 or more text editors
      2. Notice the changed layout of the toolbars
      3. Toggle it open (to multiple toolbars)
      4. Refresh the page
      5. Ensure the state of the toolbars is remembered
      6. Toggle it closed (to a single toolbar)
      7. Ensure the state of the toolbars is remembered

      Test 3

      1. Try changing the TinyMCE buttons to have only one row or more than 3 rows (Site Administration > Text Editors > TinyMCE HTML editor > General settings)
      2. Make sure the toggle icon is absent if there is only 1 row and toggles all rows except first otherwise
      Show
      Test 1 Start with an unpatched clean install Apply the patch and upgrade Ensure the toolbar config has been changed from the original default Test 2 Open a page with 1 or more text editors Notice the changed layout of the toolbars Toggle it open (to multiple toolbars) Refresh the page Ensure the state of the toolbars is remembered Toggle it closed (to a single toolbar) Ensure the state of the toolbars is remembered Test 3 Try changing the TinyMCE buttons to have only one row or more than 3 rows (Site Administration > Text Editors > TinyMCE HTML editor > General settings) Make sure the toggle icon is absent if there is only 1 row and toggles all rows except first otherwise
    • Affected Branches:
      MOODLE_25_STABLE
    • Fixed Branches:
      MOODLE_26_STABLE
    • Pull from Repository:
    • Pull Master Branch:
      wip-MDL-37565-master
    • Story Points:
      20
    • Rank:
      154
    • Sprint:
      FRONTEND Sprint 1

      Description

      I suggest few changes to make tinymce editor look better:

      • "Advanced" button that show all hidden button. Hopefully tinymce support this feature.
      • Display the essential buttons in one line only. My preferences are: Bold/Italic/link/Images/heading/lists
      • tweak the CSS to match our current standard theme in a better way than current tinymce theme.

      I'm attaching a picture of Wordpress as example. It is pleasant to look, not overwhelming and show off the media button which is actually the essential one in my opinion => it would show off the pretty Moodle file picker.

      If the advanced button feature exist in tinymce, then I don't think the issue is too hard to implement. Otherwise it may require to change Tinymce and then this improvement would not be great for maintainability.

      1. collapsed editor.png
        32 kB
      2. editor_1lin.png
        77 kB
      3. editor_3lines.png
        120 kB
      4. Screenshot_15_04_13_5_41_PM.png
        927 kB
      5. wordpress-editor.png
        41 kB

        Issue Links

          Activity

          Hide
          Michael de Raadt added a comment -

          This would complement our new icons nicely.

          Show
          Michael de Raadt added a comment - This would complement our new icons nicely.
          Hide
          Jérôme Mouneyrac added a comment -

          I'll have a go to that.

          Show
          Jérôme Mouneyrac added a comment - I'll have a go to that.
          Hide
          David Scotson added a comment -

          I believe the "full screen" button can be used to toggle between a simple and an advanced mode. Moodle 1.9 used to do this when it used a different editor called FCKEditor, but I've never figured out the exact settings required for TinyMCE to make this happen.

          I did notice the last time I updated my Moodle test site that there's a new setting for passing configuration JSON to TinyMCE, so this is perhaps already possible.

          Actually, looking into what Wordpress use, that is TinyMCE with a custom skin. And it has what they call a "kitchen sink" button to show extra options. I'd be very interested in trying to copy what they've done in Moodle if you want to work together on it.

          Show
          David Scotson added a comment - I believe the "full screen" button can be used to toggle between a simple and an advanced mode. Moodle 1.9 used to do this when it used a different editor called FCKEditor, but I've never figured out the exact settings required for TinyMCE to make this happen. I did notice the last time I updated my Moodle test site that there's a new setting for passing configuration JSON to TinyMCE, so this is perhaps already possible. Actually, looking into what Wordpress use, that is TinyMCE with a custom skin. And it has what they call a "kitchen sink" button to show extra options. I'd be very interested in trying to copy what they've done in Moodle if you want to work together on it.
          Hide
          Jérôme Mouneyrac added a comment -

          Go for it David, I won't have any time soon for that.

          Show
          Jérôme Mouneyrac added a comment - Go for it David, I won't have any time soon for that.
          Hide
          Jérôme Mouneyrac added a comment -

          I attached a screenshot of what I've quickly obtain just copying the wordpress css in chrome's element inspector.

          Show
          Jérôme Mouneyrac added a comment - I attached a screenshot of what I've quickly obtain just copying the wordpress css in chrome's element inspector.
          Hide
          Jérôme Mouneyrac added a comment -

          I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue.
          For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment

          Show
          Jérôme Mouneyrac added a comment - I will not be able to work on this issue in the immediate future. In order to create a truer sense of the state of this issue and to allow other developers to have chance to become involved, I am removing myself as the assignee of this issue. For more information, see http://docs.moodle.org/dev/Changes_to_issue_assignment
          Hide
          Martin Dougiamas added a comment -

          Yes, toggling between 1 and 3 lines of icons is the way to go. The file picker needs to be on the single line too.

          Show
          Martin Dougiamas added a comment - Yes, toggling between 1 and 3 lines of icons is the way to go. The file picker needs to be on the single line too.
          Hide
          Jason Fowler added a comment -

          Added in the toggle function, would like some feedback from the integrators to know if what I have done is the Moodle-way to do things.

          Show
          Jason Fowler added a comment - Added in the toggle function, would like some feedback from the integrators to know if what I have done is the Moodle-way to do things.
          Hide
          Damyon Wiese added a comment -

          Quick comments on the branch.

          We don't want to blindly overwrite any custom toolbar settings on upgrade, probably only do this if they have not been customised already.

          You also have the wrong @package in version.php.

          Show
          Damyon Wiese added a comment - Quick comments on the branch. We don't want to blindly overwrite any custom toolbar settings on upgrade, probably only do this if they have not been customised already. You also have the wrong @package in version.php.
          Hide
          Martin Dougiamas added a comment -

          Can you also add some new screenshots? And I might update the bug summary line, sorry for any trouble.

          Show
          Martin Dougiamas added a comment - Can you also add some new screenshots? And I might update the bug summary line, sorry for any trouble.
          Hide
          Jason Fowler added a comment -

          Thanks Damyon.

          Once I figure out how to add a separator between the new button and the first button on the toolbar, I will add screenshots.

          Marina pointed out that erasing the settings was best done if there is nothing already set already, so I am trying to figure that out.

          Show
          Jason Fowler added a comment - Thanks Damyon. Once I figure out how to add a separator between the new button and the first button on the toolbar, I will add screenshots. Marina pointed out that erasing the settings was best done if there is nothing already set already, so I am trying to figure that out.
          Hide
          Jason Fowler added a comment -

          Got the separator in there, and attached the screen shot

          Show
          Jason Fowler added a comment - Got the separator in there, and attached the screen shot
          Hide
          Jason Fowler added a comment -

          Now to experiment with the customtoolbar setting

          Show
          Jason Fowler added a comment - Now to experiment with the customtoolbar setting
          Hide
          Jason Fowler added a comment -

          Okay, that's all done now. I'd still like to look into making the editor look nicer, better icons etc, if we can. As I said before, it's a real shame to have these great new icons through out moodle, and the icons for tinyMCE look nasty...

          Show
          Jason Fowler added a comment - Okay, that's all done now. I'd still like to look into making the editor look nicer, better icons etc, if we can. As I said before, it's a real shame to have these great new icons through out moodle, and the icons for tinyMCE look nasty...
          Hide
          Jason Fowler added a comment -

          Will raise a new issue for backporting the icons from TinyMCE 4.X to TinyMCE 3.X, they look like the would go nicely with what we have in place already.

          Show
          Jason Fowler added a comment - Will raise a new issue for backporting the icons from TinyMCE 4.X to TinyMCE 3.X, they look like the would go nicely with what we have in place already.
          Hide
          Jason Fowler added a comment -

          And with that done, can I please get some one to give this a thorough peer review?

          Show
          Jason Fowler added a comment - And with that done, can I please get some one to give this a thorough peer review?
          Hide
          Damyon Wiese added a comment -

          Thanks Jason,

          This is a good solution and is better than the show/hide toggle thing (Takes the same amount of space).

          [Y] Syntax
          [Y] Output
          [Y] Whitespace
          [-] Language
          [-] Databases
          [N] Testing - Needs testing instructions
          [-] Security
          [N] Documentation - I added docs_required label. Also - when importing thirdparty code, we need to include a README to say where and when the code was taken from, any licenses, etc. Needs adding to libs/thirdpartylibs.xml + a readme_moodle.txt.
          [Y] Git
          [50%] Sanity check - My only query here is how much input has been collected on the decisions about which icons should appear in the first row. My suggestion is to remove undo/redo and add the paragraph styles drop down. There may be others with a different opinion on this. Also - are we removing the tinymce hide/show thing? I think we need an issue created for that.

          Show
          Damyon Wiese added a comment - Thanks Jason, This is a good solution and is better than the show/hide toggle thing (Takes the same amount of space). [Y] Syntax [Y] Output [Y] Whitespace [-] Language [-] Databases [N] Testing - Needs testing instructions [-] Security [N] Documentation - I added docs_required label. Also - when importing thirdparty code, we need to include a README to say where and when the code was taken from, any licenses, etc. Needs adding to libs/thirdpartylibs.xml + a readme_moodle.txt. [Y] Git [50%] Sanity check - My only query here is how much input has been collected on the decisions about which icons should appear in the first row. My suggestion is to remove undo/redo and add the paragraph styles drop down. There may be others with a different opinion on this. Also - are we removing the tinymce hide/show thing? I think we need an issue created for that.
          Hide
          Jason Fowler added a comment -

          Thanks Damyon, will get on to all that right away

          Show
          Jason Fowler added a comment - Thanks Damyon, will get on to all that right away
          Hide
          Martin Dougiamas added a comment -

          +1 for what Damyon said on removing undo and adding paragraph styles, plus add:

          • Bold and Italic, they are small and really make it look like a typical editor toolbar
          • Smileys

          That would do it I think.

          Show
          Martin Dougiamas added a comment - +1 for what Damyon said on removing undo and adding paragraph styles, plus add: Bold and Italic, they are small and really make it look like a typical editor toolbar Smileys That would do it I think.
          Hide
          Jason Fowler added a comment -

          Spoke to Martin about this at lunch, and seeing as the PDW Toggle Toolbar plugin is 1) based on the Kitchen Sink from Wordpress and 2) a plugin created for TinyMCE, he feels that we can assume that it is available under one of their licenses, and as long as we document it as such, we should be fine, giving credit where the credit is due.

          Show
          Jason Fowler added a comment - Spoke to Martin about this at lunch, and seeing as the PDW Toggle Toolbar plugin is 1) based on the Kitchen Sink from Wordpress and 2) a plugin created for TinyMCE, he feels that we can assume that it is available under one of their licenses, and as long as we document it as such, we should be fine, giving credit where the credit is due.
          Hide
          Jason Fowler added a comment -

          I have added a readme_moodle.txt as I have seen in other plugins. And moved the buttons around as per Martin's suggestion (the smiley's are optional, so I haven't moved them, although I could modify where they appear if they are turned on).

          Just need to look in to the README about where the code came from and the thirdpartylibs.xml thing

          Show
          Jason Fowler added a comment - I have added a readme_moodle.txt as I have seen in other plugins. And moved the buttons around as per Martin's suggestion (the smiley's are optional, so I haven't moved them, although I could modify where they appear if they are turned on). Just need to look in to the README about where the code came from and the thirdpartylibs.xml thing
          Hide
          Jason Fowler added a comment -

          All sorted, and now pushing for a final peer review.

          Show
          Jason Fowler added a comment - All sorted, and now pushing for a final peer review.
          Hide
          Colin Chambers added a comment -

          Had a quick look through this. Looks interesting and useful. Good work +1 from me

          Show
          Colin Chambers added a comment - Had a quick look through this. Looks interesting and useful. Good work +1 from me
          Hide
          Jason Fowler added a comment -

          I've received a response from the original author of the PDW Toggle Toolbars, and he has released his plugin under the GPL, as we had assumed, and has now uploaded a GPL'd copy to his site.

          Show
          Jason Fowler added a comment - I've received a response from the original author of the PDW Toggle Toolbars, and he has released his plugin under the GPL, as we had assumed, and has now uploaded a GPL'd copy to his site.
          Hide
          Jason Fowler added a comment -

          Sorry, not GPL, the MIT license ...

          Show
          Jason Fowler added a comment - Sorry, not GPL, the MIT license ...
          Hide
          Jason Fowler added a comment -

          All updated now to include the licensed version of the code.

          Show
          Jason Fowler added a comment - All updated now to include the licensed version of the code.
          Hide
          Jason Fowler added a comment -

          Oh, and thanks Colin.

          Show
          Jason Fowler added a comment - Oh, and thanks Colin.
          Hide
          David Scotson added a comment -

          Is there anyone looking at TinyMCE v4 for Moodle?

          http://www.tinymce.com/index.php

          Show
          David Scotson added a comment - Is there anyone looking at TinyMCE v4 for Moodle? http://www.tinymce.com/index.php
          Hide
          Petr Škoda added a comment -

          I suppose we should import tinymce 4 soon into master, it might be better to do it before this issue gets integrated.

          Show
          Petr Škoda added a comment - I suppose we should import tinymce 4 soon into master, it might be better to do it before this issue gets integrated.
          Hide
          Petr Škoda added a comment -

          Hmm, the changes in v4 are pretty big, maybe we should make a new editor plugin instead...

          Show
          Petr Škoda added a comment - Hmm, the changes in v4 are pretty big, maybe we should make a new editor plugin instead...
          Hide
          Damyon Wiese added a comment -

          Thanks for chasing down the license. This looks ready to send to integration now (sending).

          Show
          Damyon Wiese added a comment - Thanks for chasing down the license. This looks ready to send to integration now (sending).
          Hide
          Damyon Wiese added a comment -

          Actually - just spotted this branch is missing a version bump for tinymce.

          Show
          Damyon Wiese added a comment - Actually - just spotted this branch is missing a version bump for tinymce.
          Hide
          Damyon Wiese added a comment -

          Jason just added the version bump so this is fine now.

          Show
          Damyon Wiese added a comment - Jason just added the version bump so this is fine now.
          Hide
          Jason Fowler added a comment -

          Thanks Damyon.

          Petr: yeah, I wanted to bring in TinyMCE 4, but it's still in beta, so it will have to wait a while I think.

          Show
          Jason Fowler added a comment - Thanks Damyon. Petr: yeah, I wanted to bring in TinyMCE 4, but it's still in beta, so it will have to wait a while I think.
          Hide
          Petr Škoda added a comment -

          4.0.1 is not beta any more

          Show
          Petr Škoda added a comment - 4.0.1 is not beta any more
          Hide
          Jason Fowler added a comment -

          Awesome. It was when I started this issue 2 weeks ago... will look in to it as a possibility...

          Show
          Jason Fowler added a comment - Awesome. It was when I started this issue 2 weeks ago... will look in to it as a possibility...
          Hide
          Eloy Lafuente (stronk7) added a comment -

          So... is this halted, awaiting for any 4.x upgrade or no? Note it's also blocking MDL-23646

          Show
          Eloy Lafuente (stronk7) added a comment - So... is this halted, awaiting for any 4.x upgrade or no? Note it's also blocking MDL-23646
          Hide
          Jason Fowler added a comment -

          Please continue with the integration of this patch. 4.x of tiny mce will need to be integrated As a whole new editor i think... In the meantime, this needs to get in ASAP

          Show
          Jason Fowler added a comment - Please continue with the integration of this patch. 4.x of tiny mce will need to be integrated As a whole new editor i think... In the meantime, this needs to get in ASAP
          Hide
          Dan Poltawski added a comment -

          Hi Jason,

          Some small things in b/lib/editor/tinymce/plugins/pdw/lib.php:

          1. 2012 copyright I think you mean 2013.
          2. Bad whitespace (tab I think instead of a space)

          In lib/editor/tinymce/upgrade.txt, you could add the name of the plugin.

          Also, just pinging Petr Škoda to cast his eyes on it, as he has been doing most of the tinymce work in the past.

          Show
          Dan Poltawski added a comment - Hi Jason, Some small things in b/lib/editor/tinymce/plugins/pdw/lib.php: 2012 copyright I think you mean 2013. Bad whitespace (tab I think instead of a space) In lib/editor/tinymce/upgrade.txt, you could add the name of the plugin. Also, just pinging Petr Škoda to cast his eyes on it, as he has been doing most of the tinymce work in the past.
          Hide
          Dan Poltawski added a comment -

          Hi Jason,

          Actually, i'm going to need to reopen this. I've tested the upgrade twice now and it seems that your upgrade step is not working at all.

          So, please:

          1. Test and fix that
          2. Add both default settings upgrade and customised settings upgrade to the testing instructions.

          thanks,
          Dan

          Show
          Dan Poltawski added a comment - Hi Jason, Actually, i'm going to need to reopen this. I've tested the upgrade twice now and it seems that your upgrade step is not working at all. So, please: Test and fix that Add both default settings upgrade and customised settings upgrade to the testing instructions. thanks, Dan
          Hide
          CiBoT added a comment -

          Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.

          Show
          CiBoT added a comment - Moving this reopened issue out from current integration. Please, re-submit it for integration once ready.
          Hide
          Jason Fowler added a comment -

          Thanks for that Dan, will have a look at it.

          Show
          Jason Fowler added a comment - Thanks for that Dan, will have a look at it.
          Hide
          Jason Fowler added a comment -

          Okay, fixed the code.

          Show
          Jason Fowler added a comment - Okay, fixed the code.
          Hide
          Marina Glancy added a comment -

          Hi Jason,
          this is the great plugin!

          Just one comment:
          https://github.com/phalacee/moodle/compare/master...wip-MDL-37565-master#L5R28
          var $buttons is not needed here because this is to be displayed on tinymce settings page and this button is added automatically so it is only confusing there.

          Everything else looks and works fine. There is a little problem with testing instructions though. Test 1 is impossible since this issue is being integrated together with MDL-23646 and Damyon does change this setting.

          Show
          Marina Glancy added a comment - Hi Jason, this is the great plugin! Just one comment: https://github.com/phalacee/moodle/compare/master...wip-MDL-37565-master#L5R28 var $buttons is not needed here because this is to be displayed on tinymce settings page and this button is added automatically so it is only confusing there. Everything else looks and works fine. There is a little problem with testing instructions though. Test 1 is impossible since this issue is being integrated together with MDL-23646 and Damyon does change this setting.
          Hide
          Marina Glancy added a comment -

          Jason, can you please add an icon to pix folder so it is displayed on WWWROOT/admin/settings.php?section=editorsettingstinymce
          (should be 16x16)

          Show
          Marina Glancy added a comment - Jason, can you please add an icon to pix folder so it is displayed on WWWROOT/admin/settings.php?section=editorsettingstinymce (should be 16x16)
          Hide
          Jason Fowler added a comment -

          Okay, I will make those changes an push the patch back up. Thanks Marina.

          Show
          Jason Fowler added a comment - Okay, I will make those changes an push the patch back up. Thanks Marina.
          Hide
          Jason Fowler added a comment -

          changes made and pushed

          Show
          Jason Fowler added a comment - changes made and pushed
          Hide
          Marina Glancy added a comment -

          I created an issue MDL-40574 - problems with disabling tinymce plugin

          Show
          Marina Glancy added a comment - I created an issue MDL-40574 - problems with disabling tinymce plugin
          Hide
          Marina Glancy added a comment -

          Thank you Jason, this has been integrated

          Show
          Marina Glancy added a comment - Thank you Jason, this has been integrated
          Hide
          Frédéric Massart added a comment -

          Not reading through the comments in this issue, here is a quick feedback:

          • I don't think we should have the collapsible editor option any more. Clicking "Show editing tools" to see one line of buttons doesn't make sense any more.
          • I personally think we should prevent the use of some headings (especially h1, perhaps h2) as it could break the page semantic.

          It's looking great though, are we planning on designing monochrome icons that are matching our style for that?

          Show
          Frédéric Massart added a comment - Not reading through the comments in this issue, here is a quick feedback: I don't think we should have the collapsible editor option any more. Clicking "Show editing tools" to see one line of buttons doesn't make sense any more. I personally think we should prevent the use of some headings (especially h1, perhaps h2) as it could break the page semantic. It's looking great though, are we planning on designing monochrome icons that are matching our style for that?
          Hide
          Jason Fowler added a comment -

          There is an issue to introduce monochrome icons from tinymce 4.0. And there is also a plan to remove the "Show editing tools" toggle once this issue has passed testing.

          Show
          Jason Fowler added a comment - There is an issue to introduce monochrome icons from tinymce 4.0. And there is also a plan to remove the "Show editing tools" toggle once this issue has passed testing.
          Hide
          Frédéric Massart added a comment -

          \o/

          Show
          Frédéric Massart added a comment - \o/
          Hide
          Rossiani Wijaya added a comment -

          Hi Jason,

          While testing this, I noticed the width for the editor is keep changing when I toggled from 1 line to 3 lines toolbar. *see attachments.

          I think it would be better if the width stay the same for 1 to 3 lines.

          The functionality works fine though.

          Show
          Rossiani Wijaya added a comment - Hi Jason, While testing this, I noticed the width for the editor is keep changing when I toggled from 1 line to 3 lines toolbar. *see attachments. I think it would be better if the width stay the same for 1 to 3 lines. The functionality works fine though.
          Hide
          Jason Fowler added a comment -

          I can't see the attachments, but it is meant to change width, the idea is to allow it to be as small as possible...

          Show
          Jason Fowler added a comment - I can't see the attachments, but it is meant to change width, the idea is to allow it to be as small as possible...
          Hide
          David Monllaó added a comment -

          This issue is failing a behat test (http://integration.moodle.org/job/4.-%20Run%20all%20behat%20features%20(master)/28/testReport/(root)/Add%20or%20remove%20items%20from%20the%20TinyMCE%20editor%20toolbar/Add_icons/) To confirm it I've run the failed test considering this cycle's tinymce issues:

          • With MDL-23646 reverted => fails
          • With MDL-40450 reverted => fails
          • Checkout to 82c9aa843e6c45b82e272370303a3b18df8fab6b (before MDL-37565) => passes

          The problem is related with the formchangedchecker, even without changing any of the form's elements I'm asked to confirm that I want to leave the page, you can check it adding a database activity to a course for example. If there is no easy or quick solution as a dirty alternative we can introduce an extra step in the failed test to cancel the form so no JS alert is thrown, but IMO we should fix the problem before passing it.

          Show
          David Monllaó added a comment - This issue is failing a behat test ( http://integration.moodle.org/job/4.-%20Run%20all%20behat%20features%20(master)/28/testReport/(root)/Add%20or%20remove%20items%20from%20the%20TinyMCE%20editor%20toolbar/Add_icons/ ) To confirm it I've run the failed test considering this cycle's tinymce issues: With MDL-23646 reverted => fails With MDL-40450 reverted => fails Checkout to 82c9aa843e6c45b82e272370303a3b18df8fab6b (before MDL-37565 ) => passes The problem is related with the formchangedchecker, even without changing any of the form's elements I'm asked to confirm that I want to leave the page, you can check it adding a database activity to a course for example. If there is no easy or quick solution as a dirty alternative we can introduce an extra step in the failed test to cancel the form so no JS alert is thrown, but IMO we should fix the problem before passing it.
          Hide
          Rossiani Wijaya added a comment -

          oops forgot to upload the attachments.

          Show
          Rossiani Wijaya added a comment - oops forgot to upload the attachments.
          Hide
          Rossiani Wijaya added a comment -

          Sorry Jason, I have to fail this test because it is failing behat test.

          Test failed.

          Show
          Rossiani Wijaya added a comment - Sorry Jason, I have to fail this test because it is failing behat test. Test failed.
          Hide
          Marina Glancy added a comment -

          Jason I was able to reproduce it by repeating the behat test instructions:

          1. Open TinyMCE plugin setting and set layout to:
            fontselect,fontsizeselect,formatselect,|,undo,redo,|,search,replace,|,fullscreen,anchor
          2. Go to the course and Add Database activity
          3. Do not do anything and press any link on the page, the form modal dialog appears
          Show
          Marina Glancy added a comment - Jason I was able to reproduce it by repeating the behat test instructions: Open TinyMCE plugin setting and set layout to: fontselect,fontsizeselect,formatselect,|,undo,redo,|,search,replace,|,fullscreen,anchor Go to the course and Add Database activity Do not do anything and press any link on the page, the form modal dialog appears
          Hide
          Jason Fowler added a comment -

          Thanks Marina, will work that out now then.

          That's fine Rossi, thanks for testing it

          Show
          Jason Fowler added a comment - Thanks Marina, will work that out now then. That's fine Rossi, thanks for testing it
          Hide
          Marina Glancy added a comment -

          Also if there is defined only one line of buttons in tinymce editor, the second and consecutive html editors are not displayed on the page and the JS console is full of errors (I was trying to add a quiz question)

          Show
          Marina Glancy added a comment - Also if there is defined only one line of buttons in tinymce editor, the second and consecutive html editors are not displayed on the page and the JS console is full of errors (I was trying to add a quiz question)
          Hide
          Marina Glancy added a comment -

          Jason, this commit seems to fix this:
          https://github.com/marinaglancy/moodle/commit/bf50508f06cf7bfa09fbf8e92e67de537acad647
          do you mind me pulling it in?
          (Diff looks big but it's mostly because of comments and indentation)

          Show
          Marina Glancy added a comment - Jason, this commit seems to fix this: https://github.com/marinaglancy/moodle/commit/bf50508f06cf7bfa09fbf8e92e67de537acad647 do you mind me pulling it in? (Diff looks big but it's mostly because of comments and indentation)
          Hide
          Jason Fowler added a comment -

          sure, go for it Thanks Marina

          Show
          Jason Fowler added a comment - sure, go for it Thanks Marina
          Hide
          Dan Poltawski added a comment -

          I've pulled the fix in and sending back to testing.

          Show
          Dan Poltawski added a comment - I've pulled the fix in and sending back to testing.
          Hide
          Dan Poltawski added a comment -

          vendor/bin/behat --config /Users/danp/moodledata/integration_behatdata/behat/behat.yml --name "Add or remove items from the TinyMCE editor toolbar"
          .................................

          3 scenarios (3 passed)
          33 steps (33 passed)
          3m15.091s

          Show
          Dan Poltawski added a comment - vendor/bin/behat --config /Users/danp/moodledata/integration_behatdata/behat/behat.yml --name "Add or remove items from the TinyMCE editor toolbar" ................................. 3 scenarios (3 passed) 33 steps (33 passed) 3m15.091s
          Hide
          Jason Fowler added a comment -

          Sweet, thanks Dan, and thanks Marina!

          Show
          Jason Fowler added a comment - Sweet, thanks Dan, and thanks Marina!
          Hide
          David Monllaó added a comment -

          Thanks, also passing all here, the CI job is running right now, so it will report this failure, but tomorrow should pass with this last patch integrated.

          Show
          David Monllaó added a comment - Thanks, also passing all here, the CI job is running right now, so it will report this failure, but tomorrow should pass with this last patch integrated.
          Hide
          Rossiani Wijaya added a comment -

          Retested this and behat is passing now.

          Also tested, Marina's additional testing instructions and it works great.

          Passing this test.

          Show
          Rossiani Wijaya added a comment - Retested this and behat is passing now. Also tested, Marina's additional testing instructions and it works great. Passing this test.
          Hide
          Damyon Wiese added a comment -

          a single bug fix
          a drop in a waterfall
          hear the mighty roar

          Thanks for your contribution!

          Show
          Damyon Wiese added a comment - a single bug fix a drop in a waterfall hear the mighty roar Thanks for your contribution!
          Hide
          Tim Hunt added a comment -

          I am confused to find that you added a new form of editor collapsing without removing the previous editor collapsing we added in Moodle 2.5.

          What were you thinking?

          Shall I create a new issue for this regression?

          Show
          Tim Hunt added a comment - I am confused to find that you added a new form of editor collapsing without removing the previous editor collapsing we added in Moodle 2.5. What were you thinking? Shall I create a new issue for this regression?
          Hide
          Jason Fowler added a comment - - edited

          I was thinking that it would be nice to keep the "Hide editing tools" in place until we have removed the whole of the visual clutter from the TinyMCE editor. It needs to be discussed as a policy whether it is removed or not. I think Michael has raised a policy issue for this discussion to take place (it would have happened last Thursday, but Martin was away, so it will happen tonight I think).

          Additionally, with the inclusion of Damyon's Wrap plugin for TinyMCE, on narrow screens, there are still times when the toolbar is 2+ icons deep, so there is potentially a reason to keep the previous collapse method in place.

          If it is removed I would prefer to wait until the icons have been redesigned too, as to avoid visual clutter in the mean time.

          And having said all that, I don't think it's an urgent matter just yet, 2.6 is a while away from release, so we don't need to rush these decisions through.

          Show
          Jason Fowler added a comment - - edited I was thinking that it would be nice to keep the "Hide editing tools" in place until we have removed the whole of the visual clutter from the TinyMCE editor. It needs to be discussed as a policy whether it is removed or not. I think Michael has raised a policy issue for this discussion to take place (it would have happened last Thursday, but Martin was away, so it will happen tonight I think). Additionally, with the inclusion of Damyon's Wrap plugin for TinyMCE, on narrow screens, there are still times when the toolbar is 2+ icons deep, so there is potentially a reason to keep the previous collapse method in place. If it is removed I would prefer to wait until the icons have been redesigned too, as to avoid visual clutter in the mean time. And having said all that, I don't think it's an urgent matter just yet, 2.6 is a while away from release, so we don't need to rush these decisions through.
          Hide
          Jason Fowler added a comment -

          Turns out the policy issue was only created on Monday so it wouldn't have happened last Thursday. If you'd like to add your views, the issue is MDL-40668 and it was already linked to this issue.

          Show
          Jason Fowler added a comment - Turns out the policy issue was only created on Monday so it wouldn't have happened last Thursday. If you'd like to add your views, the issue is MDL-40668 and it was already linked to this issue.
          Hide
          Tim Hunt added a comment -

          Well, the fact that you were planning more work was not at all clear to me. I would expect a bunch of related work like this to be managed through a meta-bug (old style) or an Epic (new style). I just edited a question in master, and saw a bloody mess.

          Your point about narrow screen and wrap is a good one. The key example form for testing whether simplified editors are really simple enough is editing a multiple choice question. You may want to use that in your testing instructions in future.

          Show
          Tim Hunt added a comment - Well, the fact that you were planning more work was not at all clear to me. I would expect a bunch of related work like this to be managed through a meta-bug (old style) or an Epic (new style). I just edited a question in master, and saw a bloody mess. Your point about narrow screen and wrap is a good one. The key example form for testing whether simplified editors are really simple enough is editing a multiple choice question. You may want to use that in your testing instructions in future.
          Hide
          Jason Fowler added a comment -

          There are a bunch of issues linked to this one. Including the extra work and the policy issue.

          Show
          Jason Fowler added a comment - There are a bunch of issues linked to this one. Including the extra work and the policy issue.
          Hide
          Tim Hunt added a comment -

          Yes, I can see that many links exist (though some are hidden behind show more). I am simply providing you with the feedback that that does not make it at all clear what you are doing, and in other cases people have managed to do a better job of communicating to interested observers what is going on. I was hoping that you would wish to make your plans clear, and that feedback on whether you had actually managed to do that was helpful.

          Show
          Tim Hunt added a comment - Yes, I can see that many links exist (though some are hidden behind show more). I am simply providing you with the feedback that that does not make it at all clear what you are doing, and in other cases people have managed to do a better job of communicating to interested observers what is going on. I was hoping that you would wish to make your plans clear, and that feedback on whether you had actually managed to do that was helpful.
          Hide
          Jason Fowler added a comment -

          Ah, okay, good point Tim. Thanks for the feedback. I will keep that in mind for the future.

          Show
          Jason Fowler added a comment - Ah, okay, good point Tim. Thanks for the feedback. I will keep that in mind for the future.
          Hide
          Mary Cooch added a comment -

          Removing docs_required label as I have been working on http://docs.moodle.org/26/en/Text_editor

          Show
          Mary Cooch added a comment - Removing docs_required label as I have been working on http://docs.moodle.org/26/en/Text_editor

            People

            • Votes:
              5 Vote for this issue
              Watchers:
              20 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Agile