-
Notifications
You must be signed in to change notification settings - Fork 3
TASK-1828883: adding support for SimpleTableManual #94
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
Conversation
6ca1a36 to
53ab06e
Compare
|
@marekpelc-pega please keep this branch up to date with master. |
3e522b7 to
f943ba6
Compare
| import com.pega.constellation.sdk.kmp.core.internal.ComponentManagerImpl.Companion.getComponentTyped | ||
| import kotlinx.serialization.json.JsonObject | ||
|
|
||
| class ModalViewContainerComponent(context: ComponentContext) : ContainerComponent(context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It should be marked with HideableComponent interface.
| import kotlinx.serialization.json.JsonObject | ||
| import kotlinx.serialization.json.jsonObject | ||
|
|
||
| class SimpleTableManualComponent(context: ComponentContext) : BaseComponent(context) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
HideableComponent
ui-renderer-cmp/build.gradle.kts
Outdated
| implementation(libs.kotlinx.datetime) | ||
| implementation(libs.ksoup.http.parser) | ||
| implementation(libs.table.m3) | ||
| implementation("com.mohamedrejeb.dnd:compose-dnd:0.3.0") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
why hardcoded package name?
| val columnsJsonArray = props.getJSONArray("columnLabels") | ||
| return columnsJsonArray.mapWithIndex { getString(it) } | ||
| } | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
extra new line
| waitForNode("Lists group heading", substring = true) | ||
| waitForNode("List group instructions", substring = true) | ||
| waitForNode("cars") | ||
| waitForNode("Encryption keys") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why this change?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fixed that. It was passing earlier because the table was not actually loaded and we had only 1 label of the view.
Once the table gets loaded we get label twice and that caused this assert to fail (because waitForNode checks for exactly 1 element)
Repeating label is also an issue raised as ISSUE-138617
| import { Utils } from "../../helpers/utils.js"; | ||
| import { BaseComponent } from "../base.component.js"; | ||
| import {Utils} from "../../helpers/utils.js"; | ||
| import {BaseComponent} from "../base.component.js"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code formatting issue - here and in many other places in JS files
| }; | ||
| } | ||
|
|
||
| function updateFieldLabels (fields, configFields, primaryFieldsViewIndex, pConnect, options) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code formatting - extra space
| compId; | ||
| type; | ||
| props; | ||
| alive = false; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should think about making all the components to inherit from BaseComponent at some point.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
BaseComponent has pConn and action buttons and alert banner does not...
but we may think about some refactorings in the future
| ] | ||
| : []; | ||
|
|
||
| this.alertBannerComponents = banners.map((b) => |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In case there are some previous banners - should not we destroy them first?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we do that in line 247
| constructor(componentsManager, pConn) { | ||
| super(componentsManager, pConn); | ||
| this.type = "SimpleTableManual"; | ||
| this.utils = new Utils(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
utils is already initialized in the super class (BaseComponent).
tomaszmlocek-pega
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I have put some minor comments but generally changes look fine.
such update happens in dropdown component when it is created and quickly destroyed. It happens because it is loading DP from server and once its loaded props are updated on dead component - assignment component - sending empty children prop once assignmentCardComponent is null
- adding ModalViewContainerComponent
…PATCH requests one after another. Sometimes 2 POST/PATCH requests are executed one after another so quickly that setting body occures twice before the calls are executed and 2nd request has 'null' body
f943ba6 to
1fb80a9
Compare
Uh oh!
There was an error while loading. Please reload this page.