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

XWIKI-21782: Incentivize use of header rows for the WYSIWYG editor tables #3310

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

Sereza7
Copy link
Contributor

@Sereza7 Sereza7 commented Aug 1, 2024

Jira URL

https://jira.xwiki.org/browse/XWIKI-21782

Changes

Description

  • Added a translation key-value for the warning message
  • Updated the default value of the table header to top row
  • Added a warning message (style from bootstrap, consistent with the color theme) that is displayed when the user chooses to have No headers for the table.

Clarifications

  • I decided to not go further in updating the dialog style since this is quite complex and feeble. Unfortunately like we can see on the demo video below, adding the warning message moves around all the content of the dialog. We'd need an overhaul on the style of the whole tab to change that and this would be beyond the point of this change. Note that this is a bit complex since the source of this tab is in CKEditor itself and we'd rework its style by overridding a lot of properties...

Screenshots & Video

We can see on the demo video below that the new default for the table header field is First row, and selecting the None option displays a warning.
https://github.com/user-attachments/assets/fabcea6f-7c02-4bc8-9b64-266e43fefeab

Executed Tests

Looking for table in the ckeditor test module only prompted me with one significant result, which is QuickActionsIT#table.
The default state of the table changed, so I also changed the output obtained with this minimal set of inputs. After the change of the test,

mvn clean install -f xwiki-platform-core/xwiki-platform-ckeditor/xwiki-platform-ckeditor-test/xwiki-platform-ckeditor-test-docker -Dit.test=QuickActionsIT#table

successfully passed while it did fail without the changes.

Expected merging strategy

  • Prefers squash: Yes
  • Backport on branches:
    • None

…bles

* Added a translation key-value for the warning message
* Updated the default value of the table header to `top row`
* Added a warning message (style from bootstrap, consistent with the color theme) that is displayed when the user chooses to have No headers for the table.
…bles

* Updated test to take into account the new defaults
…bles

* Fixed superfluous codestyle change
* Improved the translation default value.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant