timdown / rangy

A cross-browser JavaScript range and selection library.
MIT License
2.24k stars 368 forks source link

applyToSelection on Same Selection Doesn't Nest #68

Closed timdown closed 10 years ago

timdown commented 10 years ago

From mich...@panmedia.co.nz on September 06, 2011 08:02:26

What steps will reproduce the problem? 1. Select some text.

  1. Execute the following:
var direction = 'increase';

var create_applier = function(tag) {
   return rangy.createCssClassApplier(tag, {
        normalize: true,    
        elementTagName: tag
    });
};

var remove = (direction == 'increase') ? create_applier('small') : create_applier('big');
var add = (direction == 'increase') ? create_applier('big') : create_applier('small');

if (remove.isAppliedToSelection()) remove.undoToSelection();
else add.applyToSelection();

if (remove.isAppliedToSelection()) remove.undoToSelection();
else add.applyToSelection();

if (remove.isAppliedToSelection()) remove.undoToSelection();
else add.applyToSelection(); What is the expected output? What do you see instead? Expected: selection to be wrapped in 3 nested 'big' tags

Result: selection wrapped in 1 'big' tag What version of the product are you using? On what operating system? SVN revision 486 , Linux Ubuntu 11.04 x86_64, Firefox 7 Please provide any additional information below. I'm trying to create 'increase/decrease' font-size functionality. I am aware that the 'big' tag is deprecated, but it is suitable for this example.

Original issue: http://code.google.com/p/rangy/issues/detail?id=68

timdown commented 10 years ago

From timd...@gmail.com on September 06, 2011 17:19:14

Each applier only works to disable and enable a particular class and will avoid nesting elements with the same class by design. To do what you want, you'll need a separate class for each possible font size.

timdown commented 10 years ago

From timd...@gmail.com on September 06, 2011 17:20:40

Status: NotABug

timdown commented 10 years ago

From Est.Ergo...@gmail.com on September 06, 2011 17:30:18

Yes, that is a possible solution - but it doesn't allow relative font sizes - for example, if we have this css:

.small { font-size: 90%; }

And wrap selected text in a tag each time the user attempts to increase the font size, the user is free to apply any font size they wish - instead of limiting them to n possibilities.

Would it be possible to add the option 'nest' to createCssApplier, that wraps the selection in the given tag + class if set?

e.g.

rangy.createCssClassApplier('span', { normalize: true, nest: true });

timdown commented 10 years ago

From timd...@gmail.com on September 07, 2011 16:53:10

I see what you want to do and I agree it's a useful thing to be able to do. However, the CSS class applier module simply can't do it in its current form. Your suggestion is a good one, but it will take some work and thought to implement sensibly and it's unlikely I'll add it to the CSS class applier module; it's more likely I'll create a custom command as part of the forthcoming commands module, which should provide a flexible framework for building custom commands. However, this is some way off at the moment. Sorry.

timdown commented 10 years ago

From mich...@panmedia.co.nz on September 07, 2011 17:20:52

Not a problem at all, thank you very much for your time!

Rangy has already vastly simplified the tasks I needed to accomplish, and in a much more reliable way across browsers.

Thank you for developing & releasing such a useful library!