josephtklein / jwysiwyg

Automatically exported from code.google.com/p/jwysiwyg
GNU General Public License v2.0
0 stars 0 forks source link

Order of controls and size of editor div #195

Open GoogleCodeExporter opened 9 years ago

GoogleCodeExporter commented 9 years ago
The issue here I bring up isn't really a bug issue. It's more like a discussion 
to some bits which can be done better. 

1 - Order of customised controls
==========
I realise that I could not have the controls the order I want. In the current  
version - 0.6, the default toolbar is arranged like this: bold, italic, 
strikeThrough, underline, etc... If I insert my customised control like this - 
italic, bold, underline, strikeThrough, I cannot have the controls re-ordered 
in the way I entered. 
So I changed something in the JS file to make this work:
I replace the following:
[code---
        options.controls = $.extend(true, options.controls, Wysiwyg.TOOLBAR);

        for ( var control in controls )
        {
            if ( control in options.controls )
                $.extend(options.controls[control], controls[control]);
            else
                options.controls[control] = controls[control];
        }
---code]
into something like this:
[code---
        options.controls = $.extend(true, {}, controls, Wysiwyg.TOOLBAR, controls);
---code]
I know there are two "controls" here which isn't very elegant, but at least 
it's better than a loop like above.

2 - Size of the editor div
==========
I saw these two lines in the JS file for finding out the width of the original 
textarea before replaced by the rich text box:
[code---
            var newX = element.width || element.clientWidth;
            var newY = element.height || element.clientHeight;
---code]
Please allow me to raise a question here - would it better to use 
"element.offsetWidth" than "element.width" here? I am aware of the difference 
between offsetWidth and width. offsetWidth includes all the margin, padding, 
border, etc of the element while width would exclude all these. People use 
width most of the time because they want to save the troubles, however, when 
there is a scrollbar in the textarea in the first place, the width will be 
miscalculated if using the 'width' property. The final width of the rich text 
box will be exactly one scrollbar-width smaller than the initial textarea. 
Since the scrollbar can be of any size depending on the OS and browser, it is 
difficult to add the "missing width" into the width of the final rich text box. 
I am suggesting to use offsetWidth and then deduct all margin, padding and 
border combined, then you will get the width of the textarea with the width of 
the scrollbar included. 

3 CSS value mismatched
==========
I saw a line of code to set the width of the iframe in the JS file:
[code---
width     : ( newX - 8 ).toString() + 'px'
---code]
Shouldn't it be 10 rather than 8 here? According to the default CSS, the 
padding of the wrapping div is 5, so double it (padding-left and padding-right) 
you get 10.

4 - Width of the main div
==========
Kind of relating to the previous issue... I work with XHTML-strict, so to set 
the correct width for the wrapping div, it should be:
[code---
width : ( newX > 0 ) ? ( newX - 10 ).toString() + 'px' : '100%'
---code]
rather than
[code---
width : ( newX > 0 ) ? ( newX ).toString() + 'px' : '100%'
---code]
because in XHTML-strict, the padding is not included in the width of the 
element. 

Original issue reported on code.google.com by samuelcy...@gmail.com on 28 Jun 2010 at 10:58

GoogleCodeExporter commented 9 years ago
Thanks for your information. Will be reviewed.

By the way, use latest 0.92 release from 
http://github.com/akzhan/jwysiwyg/downloads

Original comment by akzhan.a...@gmail.com on 28 Jun 2010 at 12:38