Moodle
  1. Moodle
  2. MDL-33303

Wishlist: Filemanager width should match tinymce editor width

    Details

    • Testing Instructions:
      Hide
      1. Log in and purge your caches.
      2. Create a new File resource.
      3. Make sure the File Manager has a width of 680px big enough for a couple of files.
      4. Add 6 files to the manager
      5. In maximised mode, icon view, make sure that 5 icons fit per row
      6. Make sure that list view and tree view look nice
      7. Resize the editor.
        • Make sure you can't go to small or too big.
        • Make it as big as you can, reduce the size of the page, and refresh your browser. Make sure it fits within the screen when the page loads again.
      Show
      Log in and purge your caches. Create a new File resource. Make sure the File Manager has a width of 680px big enough for a couple of files. Add 6 files to the manager In maximised mode, icon view, make sure that 5 icons fit per row Make sure that list view and tree view look nice Resize the editor. Make sure you can't go to small or too big. Make it as big as you can, reduce the size of the page, and refresh your browser. Make sure it fits within the screen when the page loads again.
    • Difficulty:
      Easy
    • Affected Branches:
      MOODLE_23_STABLE
    • Pull Master Branch:
      wip-MDL-33303-m24
    • Rank:
      41151

      Description

      I think it would look neater if the two would match?

        Issue Links

          Activity

          Hide
          Martin Dougiamas added a comment -

          + for that.

          Show
          Martin Dougiamas added a comment - + for that.
          Hide
          Frédéric Massart added a comment -

          The size of the HTML editor is based on the computed size of the textarea. By default the size will be set using the "cols" attributes which is computed differently on each browser. For instance, 80 cols in Firefox will result in 670px, 657px in Chrome or even 577px in Opera. Keeping in mind that TinyMCE keeps track of the custom size given by a user in a cookie, I thought that a static value for the file picker would do.

          Show
          Frédéric Massart added a comment - The size of the HTML editor is based on the computed size of the textarea. By default the size will be set using the "cols" attributes which is computed differently on each browser. For instance, 80 cols in Firefox will result in 670px, 657px in Chrome or even 577px in Opera. Keeping in mind that TinyMCE keeps track of the custom size given by a user in a cookie, I thought that a static value for the file picker would do.
          Hide
          Marina Glancy added a comment -

          The width of 660px leaves a wide white margin in icon view (which is default view mode), because 5 thumbnails do not fit and it is too much space for 4 thumbnails.

          I would suggest to use either 550px (for 4 thumbnails in a row) or 680px (for 5 thumbnails)

          Show
          Marina Glancy added a comment - The width of 660px leaves a wide white margin in icon view (which is default view mode), because 5 thumbnails do not fit and it is too much space for 4 thumbnails. I would suggest to use either 550px (for 4 thumbnails in a row) or 680px (for 5 thumbnails)
          Hide
          Frédéric Massart added a comment -

          Well spotted. I have set the max-width to 680px. Meantime Barbara told me about the resizable picker, I guess this property will have to be removed for the resizing to work properly.

          Show
          Frédéric Massart added a comment - Well spotted. I have set the max-width to 680px. Meantime Barbara told me about the resizable picker, I guess this property will have to be removed for the resizing to work properly.
          Hide
          Sam Hemelryk added a comment -

          Hi Fred,

          I've just been looking at this now.
          Certainly the patch constrains the size of the filemanager to something closer to the editor, however I'm not a big fan of this change by itself.
          Its interesting to read that there was some talk of resizing (although I'm not sure what talk) as I think really if we are going to implement a smaller width for the file manager I think we should implement resizing at the same time.
          This way those who want it large can still have it large... plus it makes it will bring it even closer to the editor.
          As it was nearing the end of my day I had a quick look to see if this would be easy and sure enough it is pretty simple to implement resizing in the file manager.
          I'll attach a patch shortly that gets most of the way there.
          What it doesn't do presently is the get and set user preferences in order to remember changes the user made, however I'm also not sure if thats needed. Anyway the attached patch implements resizing.

          How does everyone feel about implementing these two things at the same time (the smaller filemanager and resizing)?
          I personally feel its a more complete solution.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Fred, I've just been looking at this now. Certainly the patch constrains the size of the filemanager to something closer to the editor, however I'm not a big fan of this change by itself. Its interesting to read that there was some talk of resizing (although I'm not sure what talk) as I think really if we are going to implement a smaller width for the file manager I think we should implement resizing at the same time. This way those who want it large can still have it large... plus it makes it will bring it even closer to the editor. As it was nearing the end of my day I had a quick look to see if this would be easy and sure enough it is pretty simple to implement resizing in the file manager. I'll attach a patch shortly that gets most of the way there. What it doesn't do presently is the get and set user preferences in order to remember changes the user made, however I'm also not sure if thats needed. Anyway the attached patch implements resizing. How does everyone feel about implementing these two things at the same time (the smaller filemanager and resizing)? I personally feel its a more complete solution. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Attached patch: 33303.resize.patch

          Show
          Sam Hemelryk added a comment - Attached patch: 33303.resize.patch
          Hide
          Sam Hemelryk added a comment -

          Also just noting that while max-width of 680 is pretty good we also probably need a min-width if there isn't one.
          At less than 410px things start to wrap and overlap incorrectly.
          The attached patch uses a min resize width of 410px but that is only when resizing is enabled of course, a CSS one should be present as well.

          Show
          Sam Hemelryk added a comment - Also just noting that while max-width of 680 is pretty good we also probably need a min-width if there isn't one. At less than 410px things start to wrap and overlap incorrectly. The attached patch uses a min resize width of 410px but that is only when resizing is enabled of course, a CSS one should be present as well.
          Hide
          Frédéric Massart added a comment -

          Hi Sam,

          I have included your patch and added a resize constraint. Without the constraint the user can expand the file manager out of the window bounds which is then impossible to reduce (this is actually possible with TinyMCE as well). The patch is not yet complete but I pushed it so we can discuss it.

          • When resize is enabled, the bottom border of the file manager is hidden.
          • The scrollbar as well
          • If the user resizes its browser window, the resize constraints are not updated
          • Still needs to save the settings in a cookie
          • It is not possible to click on the 'down' arrow of the scrollbar when resize is enabled

          Perhaps adding a status bar would fix some of those things.

          Show
          Frédéric Massart added a comment - Hi Sam, I have included your patch and added a resize constraint. Without the constraint the user can expand the file manager out of the window bounds which is then impossible to reduce (this is actually possible with TinyMCE as well). The patch is not yet complete but I pushed it so we can discuss it. When resize is enabled, the bottom border of the file manager is hidden. The scrollbar as well If the user resizes its browser window, the resize constraints are not updated Still needs to save the settings in a cookie It is not possible to click on the 'down' arrow of the scrollbar when resize is enabled Perhaps adding a status bar would fix some of those things.
          Hide
          Sam Hemelryk added a comment -

          Have a look at https://github.com/samhemelryk/moodle/commit/1fd1c94
          [git fetch git://github.com/samhemelryk/moodle.git wip-MDL-33303-m23]

          It adds a status bar to separate the drag drop from the scrollbar, and adds the setting to update the user preference.
          It also tidies up a couple of things, and adds protection to make sure the filemanager fits within the page constraints.

          Unfortunately it doesn't appear that resize constraints can be adjusted dynamically (or at least with a quick look and test I found they could not through normal means). Just means that if people resize the window we can't ask the resize object to handle that, we should be able to do it ourselves however.
          Something like the following if you want to give it a shot:

          Y.on('windowresize', function(){
              if ((this.filemanager.get('offsetWidth') + this.filemanager.getX()) > this.filemanager.get('docWidth')) {
                  this.filemanager.setStyle('width', Math.round(this.filemanager.get('docWidth') - this.filemanager.getX()));
              }
          }, this);
          

          Its pretty much the same code I used to make sure the file picker fits within the page on load.
          As said it won't change the constraint width but at least it will make sure everything is visible.

          Show
          Sam Hemelryk added a comment - Have a look at https://github.com/samhemelryk/moodle/commit/1fd1c94 [git fetch git://github.com/samhemelryk/moodle.git wip-MDL-33303-m23] It adds a status bar to separate the drag drop from the scrollbar, and adds the setting to update the user preference. It also tidies up a couple of things, and adds protection to make sure the filemanager fits within the page constraints. Unfortunately it doesn't appear that resize constraints can be adjusted dynamically (or at least with a quick look and test I found they could not through normal means). Just means that if people resize the window we can't ask the resize object to handle that, we should be able to do it ourselves however. Something like the following if you want to give it a shot: Y.on('windowresize', function(){ if (( this .filemanager.get('offsetWidth') + this .filemanager.getX()) > this .filemanager.get('docWidth')) { this .filemanager.setStyle('width', Math .round( this .filemanager.get('docWidth') - this .filemanager.getX())); } }, this ); Its pretty much the same code I used to make sure the file picker fits within the page on load. As said it won't change the constraint width but at least it will make sure everything is visible.
          Hide
          Frédéric Massart added a comment -

          Hi Sam,

          I have pulled your patch and it works great! I tried to have the windowresize to work but I don't know why, it wouldn't. But if resetting the max/min width/height is not dynamically possible, the window event is not that important, especially with your code making sure the filemanager fits within the page on load.

          There is a very small thing that you might want to update, the min-height css property for .filemanager-container should be 160 instead of 140.

          I am not updating my repository. As you wrote 95% of the code, you certainly deserve the credit for this patch.

          Show
          Frédéric Massart added a comment - Hi Sam, I have pulled your patch and it works great! I tried to have the windowresize to work but I don't know why, it wouldn't. But if resetting the max/min width/height is not dynamically possible, the window event is not that important, especially with your code making sure the filemanager fits within the page on load. There is a very small thing that you might want to update, the min-height css property for .filemanager-container should be 160 instead of 140. I am not updating my repository. As you wrote 95% of the code, you certainly deserve the credit for this patch.
          Hide
          Sam Hemelryk added a comment -

          Putting this up for peer-review now

          Show
          Sam Hemelryk added a comment - Putting this up for peer-review now
          Hide
          Frédéric Massart added a comment -

          Linking with MDL-32776 which will be a duplicate as soon as Sam's patch is integrated.

          Show
          Frédéric Massart added a comment - Linking with MDL-32776 which will be a duplicate as soon as Sam's patch is integrated.
          Hide
          Sam Hemelryk added a comment -

          Updated for 2.4dev, and backported to 2.3.1 so that it can land before its release.

          Show
          Sam Hemelryk added a comment - Updated for 2.4dev, and backported to 2.3.1 so that it can land before its release.
          Hide
          Aparup Banerjee added a comment -

          the 2.2 diff url seems broken here.

          i'm really confused the commit for the master branch says

          commit 1c6cc50a3565b7712a950861d146a4b26d748d10
          Author: Frederic Massart <fred@moodle.com>

          but Sam apparently wrote 95% of code? am i looking at the right commit?

          Show
          Aparup Banerjee added a comment - the 2.2 diff url seems broken here. i'm really confused the commit for the master branch says commit 1c6cc50a3565b7712a950861d146a4b26d748d10 Author: Frederic Massart <fred@moodle.com> but Sam apparently wrote 95% of code? am i looking at the right commit?
          Hide
          Sam Hemelryk added a comment -

          Hi Apu,

          The branches has been fixed up now. There was a typo in the diff URL.
          Also just noting I had used the 22 fields for the 23 branch as when I updated this issue no-one had created the 23 fields. (I've rearranged now that the fields have been created).

          The commit went is a Fred's because I amended his commit. I can update that before integration if we want. I didn't mean to steal the issue and I'm not at all concerned about kudo so I left it as is so far as he did the initial work. Perhaps worth changing if only because then I am culpable if there are regressions

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Apu, The branches has been fixed up now. There was a typo in the diff URL. Also just noting I had used the 22 fields for the 23 branch as when I updated this issue no-one had created the 23 fields. (I've rearranged now that the fields have been created). The commit went is a Fred's because I amended his commit. I can update that before integration if we want. I didn't mean to steal the issue and I'm not at all concerned about kudo so I left it as is so far as he did the initial work. Perhaps worth changing if only because then I am culpable if there are regressions Cheers Sam
          Hide
          Aparup Banerjee added a comment -

          Thanks Sam.

          • it'd be nice to have the resize handles look the same if possible by maybe copying the look from tinymce. (or the otherway if thats possible since the filemanager resize handle looks nicer)
          • this works for me nicely and rememberer the size preference well too.
          • author wise - i think its pretty clear now in this MDL so no worries here.

          so i think this is good for integration.

          Show
          Aparup Banerjee added a comment - Thanks Sam. it'd be nice to have the resize handles look the same if possible by maybe copying the look from tinymce. (or the otherway if thats possible since the filemanager resize handle looks nicer) this works for me nicely and rememberer the size preference well too. author wise - i think its pretty clear now in this MDL so no worries here. so i think this is good for integration.
          Hide
          Sam Hemelryk added a comment -

          Great thanks Apu, putting this up for integration now.
          In regards to the image used for the handle, I don't think it would be wise to have our code reply on an image in tiny mce (prone to break in the future).
          We could easily however make their handle look like ours. They do the same thing YUI/we do and specify the handle image as a css background image so it would simply be a case of overriding it in CSS.
          Either way however I think that is best done as a separate issue. This issue changes only the filemanager presently and I think its best kept that way.
          I'll create another issue for that shortly.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Great thanks Apu, putting this up for integration now. In regards to the image used for the handle, I don't think it would be wise to have our code reply on an image in tiny mce (prone to break in the future). We could easily however make their handle look like ours. They do the same thing YUI/we do and specify the handle image as a css background image so it would simply be a case of overriding it in CSS. Either way however I think that is best done as a separate issue. This issue changes only the filemanager presently and I think its best kept that way. I'll create another issue for that shortly. Cheers Sam
          Hide
          Sam Hemelryk added a comment -

          Linked to MDL-34071 the issue I created to implement consistent resize images.

          Show
          Sam Hemelryk added a comment - Linked to MDL-34071 the issue I created to implement consistent resize images.
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Ho, about this...

          is it about to do the editor and the file manager shown, for example, in "Adding/updating a file to section XX" (resource modedit), to be the same width? I've tried here both with Safari and Firefox and I don't get it all all.

          Or is it exclusively about the "default" width (without using the bottom-left icon to modify both editor and/or filemanager sizes?

          Also, curiously, I don't get any TinyXXXX cookie here (apparently). And the editor resize seems to happen, "remembering" the last I used.

          Show
          Eloy Lafuente (stronk7) added a comment - Ho, about this... is it about to do the editor and the file manager shown, for example, in "Adding/updating a file to section XX" (resource modedit), to be the same width? I've tried here both with Safari and Firefox and I don't get it all all. Or is it exclusively about the "default" width (without using the bottom-left icon to modify both editor and/or filemanager sizes? Also, curiously, I don't get any TinyXXXX cookie here (apparently). And the editor resize seems to happen, "remembering" the last I used.
          Hide
          Sam Hemelryk added a comment -

          Hi Eloy,

          For the time being I've decided not to update the title and description of this issue (I will update the test instructions shortly).
          This issue has evolved into more of a new feature than any thing else, and perhaps best for master only but I will let you decide that.

          The file manager has been resized smaller by default by my changes however it that is not all that has gone on.
          We decided it wasn't a great option to do that by itself as in situations where you will have many files it is nice to have the larger file picker and it all gets complicated anyway when you start working with small resolutions + mobile browsers etc.
          In the end we decided the best option was a two part option:

          a. Reduce the size of the file manager from 100% to 680px which was a more reasonable size for most uses.
          b. Make the file manager resizeable like the editor and other elements so that the user can drag it in or out if they like.

          The changes purposed here do just that.
          It does one more thing on top as well (trivial) setting a minimum width for the filemanager so you don't lose it completely on tiny browsers.

          Hopefully that + the updated test instructions answer your questions.

          Cheers
          Sam

          Show
          Sam Hemelryk added a comment - Hi Eloy, For the time being I've decided not to update the title and description of this issue (I will update the test instructions shortly). This issue has evolved into more of a new feature than any thing else, and perhaps best for master only but I will let you decide that. The file manager has been resized smaller by default by my changes however it that is not all that has gone on. We decided it wasn't a great option to do that by itself as in situations where you will have many files it is nice to have the larger file picker and it all gets complicated anyway when you start working with small resolutions + mobile browsers etc. In the end we decided the best option was a two part option: a. Reduce the size of the file manager from 100% to 680px which was a more reasonable size for most uses. b. Make the file manager resizeable like the editor and other elements so that the user can drag it in or out if they like. The changes purposed here do just that. It does one more thing on top as well (trivial) setting a minimum width for the filemanager so you don't lose it completely on tiny browsers. Hopefully that + the updated test instructions answer your questions. Cheers Sam
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Aha, oki, that match what I was getting when trying it (smaller-fixed size and resizable down to a limit). Thanks for the clarification, it was getting me crazy!

          Show
          Eloy Lafuente (stronk7) added a comment - Aha, oki, that match what I was getting when trying it (smaller-fixed size and resizable down to a limit). Thanks for the clarification, it was getting me crazy!
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Going to integrate this right now... just one tiny comment for you to amend if necessary:

          Testing #7: Resize the editor.

          Is it the editor or the file manager?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Going to integrate this right now... just one tiny comment for you to amend if necessary: Testing #7: Resize the editor . Is it the editor or the file manager? Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Integrated (23 and master), thanks!

          Show
          Eloy Lafuente (stronk7) added a comment - Integrated (23 and master), thanks!
          Hide
          Jason Fowler added a comment -

          looks great to me

          Show
          Jason Fowler added a comment - looks great to me
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Uhm... it seems this is affecting also to how big popups are here and there, see:

          http://tracker.moodle.org/browse/MDL-33542?focusedCommentId=166994&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-166994

          So perhaps we need to identify a bit better which ones need to have those widths applied?

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Uhm... it seems this is affecting also to how big popups are here and there, see: http://tracker.moodle.org/browse/MDL-33542?focusedCommentId=166994&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-166994 So perhaps we need to identify a bit better which ones need to have those widths applied? Ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Also, I'm getting some strange behavior, for example going to one folder resource and clicking edit to access to the file-manager (note it's one page with a manager and without any editor).

          Initially I get it very big (width) and when I click on the resize handle, it bumps, radically to an smaller size, not giving me any opportunity to drag the handle.

          So it seems that this would need still a bit more of love... ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Also, I'm getting some strange behavior, for example going to one folder resource and clicking edit to access to the file-manager (note it's one page with a manager and without any editor). Initially I get it very big (width) and when I click on the resize handle, it bumps, radically to an smaller size, not giving me any opportunity to drag the handle. So it seems that this would need still a bit more of love... ciao
          Hide
          Eloy Lafuente (stronk7) added a comment -

          Finally agreed to revert and reopen it, because:

          1) there was some annoyances (jumps) here and there.
          2) it had too generic css selectors causing mess in various popups (MDL-33542).

          Ciao

          Show
          Eloy Lafuente (stronk7) added a comment - Finally agreed to revert and reopen it, because: 1) there was some annoyances (jumps) here and there. 2) it had too generic css selectors causing mess in various popups ( MDL-33542 ). Ciao
          Hide
          Jason Fowler added a comment -

          will need some new test instructions to cover off these overlaps...

          Show
          Jason Fowler added a comment - will need some new test instructions to cover off these overlaps...
          Hide
          Michael de Raadt added a comment -

          Taking this out of the Stable Sprint so that it can be worked on when the Sprint version is archived.

          Show
          Michael de Raadt added a comment - Taking this out of the Stable Sprint so that it can be worked on when the Sprint version is archived.
          Hide
          Michael de Raadt added a comment -

          This looks like it was almost finished. I've put it on the FRONTEND backlog and hopefully Sam will be able to continue working on it in a FRONTEND sprint.

          Show
          Michael de Raadt added a comment - This looks like it was almost finished. I've put it on the FRONTEND backlog and hopefully Sam will be able to continue working on it in a FRONTEND sprint.

            People

            • Votes:
              0 Vote for this issue
              Watchers:
              2 Start watching this issue

              Dates

              • Created:
                Updated: