N°9807 - Polishing CKEditor for iTop 3.3.0-beta1#976
Open
bdalsass wants to merge 9 commits into
Open
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() method returning the PHP class short name, and replaces GetType() (localized) with it for the attribute-type form-field metadata. |
| application/cmdbabstract.class.inc.php | Switches atttype/attribute-type data attributes from GetType() (localized) to GetTypeShortClassName() in two call sites. |
| css/backoffice/components/_form.scss | SDK form styles (form-buttons, form-error, ts-control, etc.) are fully commented out as WIP; active templates still reference these classes. |
| css/backoffice/vendors/ckeditor/_overloads.scss | Extends 200 px height rule to .ck-source-editing-area and removes the old full-screen-scoped overflow !important rule. |
| node_modules/ckeditor5-itop-build/build/styles/compiled-theme.scss | Updated compiled theme adds .ck-source-editing-area to the full-screen height rule and introduces a global overflow: auto for the source-editing textarea. |
| sources/Application/UI/Base/Component/DataTable/DataTableUIBlockFactory.php | Updates two column-definition arrays to use GetTypeShortClassName() for attribute_type. |
| sources/Renderer/Bootstrap/FieldRenderer/BsLinkedSetFieldRenderer.php | Switches attribute_type to GetTypeShortClassName() in linked-set renderer metadata array. |
| datamodels/2.x/itop-portal-base/portal/src/Controller/ManageBrickController.php | Switches sAttType to GetTypeShortClassName() in manage-brick column data. |
| datamodels/2.x/itop-portal-base/portal/src/Controller/ObjectController.php | Switches attribute_type to GetTypeShortClassName() in two portal object controller response arrays. |
| datamodels/2.x/itop-portal-base/portal/src/Helper/BrowseBrickHelper.php | Switches attribute_type to GetTypeShortClassName() in two browse-brick field data arrays. |
| datamodels/2.x/itop-portal-base/portal/public/css/themes/vendors/_ckeditor.scss | Adds .ck-source-editing-area { height: 180px } for the portal context. |
Flowchart
%%{init: {'theme': 'neutral'}}%%
flowchart TD
A["AttributeDefinition::GetType()\nReturns Dict::S('Core:ClassName')\ne.g. 'HTML', 'Text', 'Externer Schlüssel'"]
B["AttributeDefinition::GetTypeShortClassName()\nReturns ReflectionClass shortName\ne.g. 'AttributeHTML', 'AttributeText'"]
C["cmdbabstract / DataTableUIBlockFactory\nBsLinkedSetFieldRenderer / Portal files\nattribute_type in output"]
D["HTML data-attribute-type attribute"]
E["toolbox.js selector\n[data-attribute-type='AttributeHTML']\n[data-attribute-type='AttributeText']"]
F["✅ Match — highlight.js rendering applied"]
G["❌ No match — JS selector never fires (old bug)"]
A -->|"Old code"| C
B -->|"New code (this PR)"| C
C --> D
D -->|"New: 'AttributeHTML'"| E --> F
D -->|"Old: 'HTML' localized"| G
%%{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"}}}%%
flowchart TD
A["AttributeDefinition::GetType()\nReturns Dict::S('Core:ClassName')\ne.g. 'HTML', 'Text', 'Externer Schlüssel'"]
B["AttributeDefinition::GetTypeShortClassName()\nReturns ReflectionClass shortName\ne.g. 'AttributeHTML', 'AttributeText'"]
C["cmdbabstract / DataTableUIBlockFactory\nBsLinkedSetFieldRenderer / Portal files\nattribute_type in output"]
D["HTML data-attribute-type attribute"]
E["toolbox.js selector\n[data-attribute-type='AttributeHTML']\n[data-attribute-type='AttributeText']"]
F["✅ Match — highlight.js rendering applied"]
G["❌ No match — JS selector never fires (old bug)"]
A -->|"Old code"| C
B -->|"New code (this PR)"| C
C --> D
D -->|"New: 'AttributeHTML'"| E --> F
D -->|"Old: 'HTML' localized"| G
Reviews (2): 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
- restore setup css
steffunky
approved these changes
Jul 20, 2026
bdalsass
marked this pull request as ready for review
July 21, 2026 08:13
Comment on lines
+11
to
+81
| // WIP SDK Forms | ||
| //form[is="itop-form-element"] { | ||
| // | ||
| // .help-text{ | ||
| // padding: 1px 5px; | ||
| // background-color: #d7e3f8; | ||
| // border: 1px solid #c6e7f5; | ||
| // border-radius: 5px; | ||
| // margin: 5px 0; | ||
| // font-size: 0.9em; | ||
| // } | ||
| // | ||
| // .form-error ul{ | ||
| // padding: 1px 5px; | ||
| // background-color: #f8d7da; | ||
| // border: 1px solid #f5c6cb; | ||
| // border-radius: 5px; | ||
| // margin: 5px 0; | ||
| // font-size: 0.9em; | ||
| // } | ||
| // | ||
| // .subform{ | ||
| // background-color: #efefef; | ||
| // border-radius: 5px; | ||
| // padding: 10px; | ||
| // } | ||
| // | ||
| // .form-buttons{ | ||
| // margin: 20px 0; | ||
| // } | ||
| // | ||
| // .form select{ | ||
| // padding: 0; | ||
| // overflow-y: auto; | ||
| // } | ||
| // | ||
| // .form select option{ | ||
| // height: 30px; | ||
| // display: flex; | ||
| // align-items: center; | ||
| // } | ||
| // | ||
| // .turbo-refreshing{ | ||
| // opacity: .5; | ||
| // } | ||
| // | ||
| // .ibo-field legend{ | ||
| // margin-top: 24px; | ||
| // } | ||
| // | ||
| // 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-actions > .ibo-button > span{ | ||
| // margin-right: 5px; | ||
| // } | ||
| // | ||
| // .ibo-form textarea{ | ||
| // resize: vertical; | ||
| // } | ||
| // | ||
| //} |
Contributor
There was a problem hiding this comment.
.form-buttons (margin: 20px 0) and .form-error ul (error pill styling). Both classes are actively emitted by templates/application/forms/itop_form.html.twig and templates/application/forms/itop_console_layout.html.twig respectively. With these styles gone, form action buttons will lose their vertical margin and error messages will lose their red pill background. If the SDK Forms feature is deferred, consider restoring at least those two rules as globals until the SDK Forms work is complete.
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