N°9807 - Polishing CKEditor for iTop 3.3.0-beta1#976
Draft
bdalsass wants to merge 8 commits into
Draft
Conversation
- create new function in attribute definition to get the short classname of the attribute - fix iTop attribute class fields data injection
- Protect form sdk css class collisions
- fix height of `ck-source-editing-area` to be inlined with `ck-editor__editable_inline`
- fix height in portal
- apply overflow auto to ck-source-editing-area via ckeditor build
- update ckeditor build from npm
- update compiled css files
Contributor
There was a problem hiding this comment.
Pull request overview
Polishes CKEditor source/fullscreen behavior and makes attribute metadata independent of localization.
Changes:
- Uses attribute class names instead of translated type labels.
- Updates CKEditor dependency and source-editing styles.
- Scopes back-office form styles.
Reviewed changes
Copilot reviewed 10 out of 15 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
sources/Renderer/Bootstrap/FieldRenderer/BsLinkedSetFieldRenderer.php |
Stabilizes linked-set metadata. |
sources/Core/AttributeDefinition/AttributeDefinition.php |
Adds short class-name API. |
sources/Application/UI/Base/Component/DataTable/DataTableUIBlockFactory.php |
Stabilizes table metadata. |
package-lock.json |
Updates CKEditor dependency commit. |
node_modules/ckeditor5-itop-build/build/styles/compiled-theme.scss |
Updates bundled editor styles. |
node_modules/.package-lock.json |
Synchronizes installed dependency lock. |
datamodels/2.x/itop-portal-base/portal/src/Helper/BrowseBrickHelper.php |
Stabilizes browse metadata. |
datamodels/2.x/itop-portal-base/portal/src/Controller/ObjectController.php |
Stabilizes object metadata. |
datamodels/2.x/itop-portal-base/portal/src/Controller/ManageBrickController.php |
Stabilizes manage-brick metadata. |
datamodels/2.x/itop-portal-base/portal/public/css/themes/vendors/_ckeditor.scss |
Sizes portal source editor. |
datamodels/2.x/itop-portal-base/portal/public/css/bootstrap-theme-combodo.css |
Regenerates portal CSS. |
css/backoffice/vendors/ckeditor/_overloads.scss |
Adjusts source-editor sizing. |
css/backoffice/components/_form.scss |
Scopes form styling. |
application/cmdbabstract.class.inc.php |
Stabilizes legacy metadata. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+41
to
+44
| .form select{ | ||
| padding: 0; | ||
| overflow-y: auto; | ||
| } |
Comment on lines
+76
to
+78
| .ibo-form textarea{ | ||
| resize: vertical; | ||
| } |
| // Metadata | ||
| $oFormField->AddMetadata('attribute-code', $this->GetCode()); | ||
| $oFormField->AddMetadata('attribute-type', $this->GetType()); | ||
| $oFormField->AddMetadata('attribute-type', $this->GetTypeShortClassName()); |
bdalsass
changed the title
Contributor
|
| Filename | Overview |
|---|---|
| sources/Core/AttributeDefinition/AttributeDefinition.php | Adds GetTypeShortClassName() returning the raw PHP class short name (e.g. AttributeHTML) and replaces GetType() (locale-translated dictionary value) in the attribute-type metadata; aligns PHP output with what existing JS selectors already expect. |
| application/cmdbabstract.class.inc.php | Two call-sites updated from GetType() to GetTypeShortClassName() for the atttype and attribute-type fields — straightforward mechanical change consistent with the rest of the PR. |
| css/backoffice/components/_form.scss | All backoffice form utility styles scoped under form[is="itop-form-element"] to prevent CKEditor styling bleed; risks silently dropping styles for Tom Select controls or form elements rendered outside that custom-element form. |
| sources/Application/UI/Base/Component/DataTable/DataTableUIBlockFactory.php | Two call-sites migrated from GetType() to GetTypeShortClassName() for attribute_type column metadata — no logic change, purely formatting alignment. |
| node_modules/ckeditor5-itop-build/build/styles/compiled-theme.scss | Updated CKEditor build: adds fullscreen source-editing area to the full-height rule and adds overflow: auto to .ck-source-editing-area textarea globally (previously only for fullscreen via !important). |
| package-lock.json | Updates ckeditor5-itop-build reference from commit 21b6edc to 8d8e90d; from URL format changed from github: shorthand to explicit https:// URL. |
Sequence Diagram
%%{init: {'theme': 'neutral'}}%%
sequenceDiagram
participant PHP as PHP (AttributeDefinition)
participant Twig as Twig Template
participant DOM as HTML DOM
participant JS as toolbox.js
Note over PHP,JS: Before this PR
PHP->>Twig: GetType() returns "HTML" (Dict translated)
Twig->>DOM: "data-attribute-type="HTML""
JS--xDOM: "querySelector([data-attribute-type="AttributeHTML"]) no match"
Note over PHP,JS: After this PR
PHP->>Twig: GetTypeShortClassName() returns "AttributeHTML"
Twig->>DOM: "data-attribute-type="AttributeHTML""
JS->>DOM: "querySelector([data-attribute-type="AttributeHTML"]) match"
%%{init: {'theme': 'base', 'themeVariables': {"darkMode": true, "background": "#0d1117", "primaryColor": "#21262d", "primaryTextColor": "#e6edf3", "primaryBorderColor": "#8b949e", "lineColor": "#8b949e", "textColor": "#e6edf3", "edgeLabelBackground": "#161b22", "actorBkg": "#21262d", "actorBorder": "#8b949e", "actorTextColor": "#e6edf3", "actorLineColor": "#8b949e", "signalColor": "#8b949e", "signalTextColor": "#e6edf3", "noteBkgColor": "#373320", "noteBorderColor": "#d4a72c", "noteTextColor": "#f0e6c0", "labelBoxBkgColor": "#21262d", "labelBoxBorderColor": "#8b949e", "labelTextColor": "#e6edf3", "loopTextColor": "#e6edf3", "activationBkgColor": "#30363d", "activationBorderColor": "#8b949e"}}}%%
sequenceDiagram
participant PHP as PHP (AttributeDefinition)
participant Twig as Twig Template
participant DOM as HTML DOM
participant JS as toolbox.js
Note over PHP,JS: Before this PR
PHP->>Twig: GetType() returns "HTML" (Dict translated)
Twig->>DOM: "data-attribute-type="HTML""
JS--xDOM: "querySelector([data-attribute-type="AttributeHTML"]) no match"
Note over PHP,JS: After this PR
PHP->>Twig: GetTypeShortClassName() returns "AttributeHTML"
Twig->>DOM: "data-attribute-type="AttributeHTML""
JS->>DOM: "querySelector([data-attribute-type="AttributeHTML"]) match"
Reviews (1): Last reviewed commit: "N°9807 - Polishing CKEditor for iTop 3.3..." | Re-trigger Greptile
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
Comment on lines
+8
to
+80
| padding-bottom: 14px; | ||
| } | ||
|
|
||
| .help-text{ | ||
| padding: 1px 5px; | ||
| background-color: #d7e3f8; | ||
| border: 1px solid #c6e7f5; | ||
| border-radius: 5px; | ||
| margin: 5px 0; | ||
| font-size: 0.9em; | ||
| } | ||
| form[is="itop-form-element"] { | ||
|
|
||
| .form-error ul{ | ||
| padding: 1px 5px; | ||
| background-color: #f8d7da; | ||
| border: 1px solid #f5c6cb; | ||
| border-radius: 5px; | ||
| margin: 5px 0; | ||
| font-size: 0.9em; | ||
| } | ||
| .help-text{ | ||
| padding: 1px 5px; | ||
| background-color: #d7e3f8; | ||
| border: 1px solid #c6e7f5; | ||
| border-radius: 5px; | ||
| margin: 5px 0; | ||
| font-size: 0.9em; | ||
| } | ||
|
|
||
| .subform{ | ||
| background-color: #efefef; | ||
| border-radius: 5px; | ||
| padding: 10px; | ||
| } | ||
| .form-error ul{ | ||
| padding: 1px 5px; | ||
| background-color: #f8d7da; | ||
| border: 1px solid #f5c6cb; | ||
| border-radius: 5px; | ||
| margin: 5px 0; | ||
| font-size: 0.9em; | ||
| } | ||
|
|
||
| .form-buttons{ | ||
| margin: 20px 0; | ||
| } | ||
| .subform{ | ||
| background-color: #efefef; | ||
| border-radius: 5px; | ||
| padding: 10px; | ||
| } | ||
|
|
||
| .form select{ | ||
| padding: 0; | ||
| overflow-y: auto; | ||
| } | ||
| .form-buttons{ | ||
| margin: 20px 0; | ||
| } | ||
|
|
||
| .form select option{ | ||
| height: 30px; | ||
| display: flex; | ||
| align-items: center; | ||
| } | ||
| .form select{ | ||
| padding: 0; | ||
| overflow-y: auto; | ||
| } | ||
|
|
||
| .turbo-refreshing{ | ||
| opacity: .5; | ||
| } | ||
| .form select option{ | ||
| height: 30px; | ||
| display: flex; | ||
| align-items: center; | ||
| } | ||
|
|
||
| .ibo-field legend{ | ||
| margin-top: 24px; | ||
| } | ||
| .turbo-refreshing{ | ||
| opacity: .5; | ||
| } | ||
|
|
||
| collection-entry-element { | ||
| margin-top: 8px; | ||
| display: block; | ||
| padding: 10px 10px; | ||
| background-color: #f5f5f5; | ||
| border-radius: 5px; | ||
| } | ||
| .ts-control{ | ||
| height: auto; | ||
| min-height: 30px; | ||
| } | ||
| .ibo-field legend{ | ||
| margin-top: 24px; | ||
| } | ||
|
|
||
| .ibo-form-actions > .ibo-button > span{ | ||
| margin-right: 5px; | ||
| } | ||
| collection-entry-element { | ||
| margin-top: 8px; | ||
| display: block; | ||
| padding: 10px 10px; | ||
| background-color: #f5f5f5; | ||
| border-radius: 5px; | ||
| } | ||
| .ts-control{ | ||
| height: auto; | ||
| min-height: 30px; | ||
| } | ||
|
|
||
| .ibo-form textarea{ | ||
| resize: vertical; | ||
| } | ||
| .ibo-form-actions > .ibo-button > span{ | ||
| margin-right: 5px; | ||
| } | ||
|
|
||
| .ibo-form textarea{ | ||
| resize: vertical; | ||
| } | ||
|
|
||
| } |
Contributor
There was a problem hiding this comment.
itop-form-element forms
All form utility classes (.ts-control, collection-entry-element, .help-text, .form-error ul, .form-buttons, etc.) are now scoped under form[is="itop-form-element"]. Any Tom Select widget or form-related component rendered outside that specific custom-element form (e.g. search panels, inline edit dialogs, legacy forms) will silently lose its styling — ts-control would lose its height: auto; min-height: 30px override and potentially collapse. The choices_element.js component injects .ts-control wherever a <choices-element> appears, and not every such occurrence may be a descendant of a form[is="itop-form-element"].
- comment form sdk css classes
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Base information
Symptom (bug) / Objective (enhancement)
see corresponding ticket
Checklist before requesting a review