Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Buttons state not preserved #533

Closed
qasid opened this issue Jun 1, 2016 · 9 comments
Closed

Buttons state not preserved #533

qasid opened this issue Jun 1, 2016 · 9 comments

Comments

@qasid
Copy link

qasid commented Jun 1, 2016

I'm using following configurations of basic styles in config.js. Problem i'm having is when a style (e.g "bold") is applied, button's pressed state is not preserved. I've gone through your code and observed isactive() in buttonStateClasses returns false therefore fails to add state class "ae-button-pressed". This problem also arises when using "ae_buttonbridge" and use basic styles external plugin.

config.coreStyles_bold = 
    {
        element: 'span',
        attributes:{'style':'font-weight: bold;'},
        overrides: 'b'
    };

    config.coreStyles_italic = 
    {
        element: 'span',
        attributes:{'style':'font-style: italic;'},
        overrides: 'i'
    };

    config.coreStyles_underline = 
    {
        element: 'span',
        attributes:{'style':'text-decoration: underline;'}
    };
@ipeychev
Copy link
Contributor

ipeychev commented Jun 6, 2016

Hey @qasid,

I'm not sure what you mean with "state is not preserved". AlloyEditor does not preserve the state of buttons and it shouldn't do it. When the button is (re)rendered, its current state - pressed or not, is being determined depending on the selection.
I'm also not sure what is the problem here at all. Could you please add some reproducible code?

Thanks,

@qasid
Copy link
Author

qasid commented Jun 7, 2016

Hello @ipeychev ,

You can reproduce this scenario by adding the above config.coreStyles settings in config.js (lib/ck-editor).

As you can see below bold button is not shown pressed (using config.js) even though the text selected is bold.

button state

i hope this image clarifies.

@ipeychev
Copy link
Contributor

ipeychev commented Jun 7, 2016

Hey @qasid,

I see. Such kind of configurations we never tried so I don't have the answer on top of my head.
I think it might be because button bold has strong as style. You may try to change this property following the instructions here.

Thanks,

@jbalsas
Copy link
Contributor

jbalsas commented Jun 7, 2016

Hey @qasid, as @ipeychev hinted, this is a cas of misconfiguration. We use the button style prop to check the selection state, so those need to be in sync.

I've tried your sample an I got it working like this:

var customBoldStyle = {
    element: 'span',
    attributes: {'style': 'font-weight: bold;'},
    overrides: 'b'
};

AlloyEditor.editable('editable', {
    buttonCfg: {
        bold: {
            style: customBoldStyle
        }
    },
    coreStyles_bold: customBoldStyle
});

Hope it helps

@jbalsas
Copy link
Contributor

jbalsas commented Jun 7, 2016

On a related note, I think we can change button-style.js to:

var style = AlloyEditor.Lang.isObject(this.props.style) ? this.props.style : this.props.editor.get('nativeEditor').config[this.props.style];

this._style = new CKEDITOR.style(style);

This way we can have our buttons default to the coreStyle_* styles and we no longer need to worry about them getting out of sync.

What do you think about this, @ipeychev?

@ipeychev
Copy link
Contributor

ipeychev commented Jun 7, 2016

Hey Chema,

Yeah, I think it would be a good improvement.

Also, for the record, it would be better to overwrite button bold style property following this approach.

Thanks,

@ipeychev ipeychev closed this as completed Jun 7, 2016
@ipeychev ipeychev reopened this Jun 7, 2016
@jbalsas
Copy link
Contributor

jbalsas commented Jun 7, 2016

Also, for the record, it would be better to overwrite button bold style property following this approach.

Oh, I forgot we added that.. just updated my previous comment to reflect it 😉

@ipeychev
Copy link
Contributor

ipeychev commented Jun 7, 2016

Great, Chema, thanks!

@qasid
Copy link
Author

qasid commented Jun 8, 2016

Hello @jbalsas and @ipeychev ,

Thanks a lot it's working now. :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants