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

EZP-31380: Disable manage rows/cells buttons for table and row toolbars #1232

Merged

Conversation

SerheyDolgushev
Copy link
Contributor

Question Answer
Tickets EZP-31380
Bug fix? yes
New feature? no
BC breaks? no
Tests pass? yes
Doc needed? no
License GPL-2.0

AlloyEdior allows running the commands to manipulate the table content only when active element is td or th. So there is no need to have "insert rows"/"insert columns" buttons in "table" and "table row" toolbars.

Steps to reproduce:

  1. Insert a table using the Online Editor
  2. Using elements path click on "table" or "tr"
  3. Try to insert new rows/cells

Current results:
No new rows/cells are inserted

Expected result:
New rows/cells will be inserted

This happens because of https://github.com/liferay/alloy-editor/blob/master/src/plugins/tabletools.js#L821

Checklist:

  • Coding standards ($ composer fix-cs)
  • Ready for Code Review


export default class EzTableCellConfig extends EzConfigTableBase {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be good if the tableCell and tableHeader would still extend the EzConfigTableBase.
You can do something like this to override the buttons array:

constructor(config) {
    super(config);
    
    const editAttributesButton = config.attributes[this.name] || config.classes[this.name] ? `${this.name}edit` : '';

    this.buttons = [
        'ezmoveup',
        'ezmovedown',
        editAttributesButton,
        'tableHeading',
        'ezembedinline',
        'ezanchor',
        'eztablerow',
        'eztablecolumn',
        'eztablecell',
        'eztableremove',
        ...config.extraButtons[this.name],
      ];
}

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dew326 can you please check 3b940b7? Table cells extends basic table config, and table header extends table cell. Please let me know your thoughts on this one.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach.


export default class EzTableCellConfig extends EzConfigTableBase {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like this approach.

@dew326
Copy link
Member

dew326 commented Feb 14, 2020

@SerheyDolgushev can you open PR for 3.0 in ezplatform-richtext?

@SerheyDolgushev
Copy link
Contributor Author

Doing that right now

@SerheyDolgushev
Copy link
Contributor Author

@dew326 done ezsystems/ezplatform-richtext#113

@lserwatka lserwatka merged commit cfc9e9d into ezsystems:1.5 Feb 18, 2020
@lserwatka
Copy link
Member

@dew326 could you merge it up?

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

Successfully merging this pull request may close these issues.

6 participants