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

fix(react-components): Fix PointCloudFilterCommand because it didn't show up in Search #4703

Merged
merged 7 commits into from
Aug 19, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -91,31 +91,35 @@ export abstract class BaseFilterCommand extends RenderTargetCommand {

public getSelectedLabel(translate: TranslateDelegate): string {
if (this._children === undefined) {
return this.getNoneLabel(translate);
return BaseFilterCommand.getNoneString(translate);
}
const selected = this._children.filter((child) => child.isChecked);
const counter = selected.length;
if (counter === 0) {
return this.getNoneLabel(translate);
return BaseFilterCommand.getNoneString(translate);
}
if (counter === this._children.length) {
return this.getAllLabel(translate);
return BaseFilterCommand.getAllString(translate);
}
if (counter === 1) {
return selected[0].getLabel(translate);
}
return counter.toString() + ' ' + this.getSelected(translate);
return counter.toString() + ' ' + BaseFilterCommand.getSelectedString(translate);
}

public getAllLabel(translate: TranslateDelegate): string {
// ==================================================
// STATIC METHODS
// ==================================================

public static getAllString(translate: TranslateDelegate): string {
return translate('ALL', 'All');
}

public getNoneLabel(translate: TranslateDelegate): string {
private static getNoneString(translate: TranslateDelegate): string {
return translate('NONE', 'None');
}

public getSelected(translate: TranslateDelegate): string {
private static getSelectedString(translate: TranslateDelegate): string {
return translate('SELECTED', 'Selected');
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ export class MockFilterCommand extends BaseFilterCommand {

private _timeStamp: number | undefined = undefined;
private _useAllColor: boolean = true;
private readonly _testDynamic: boolean = false;
private readonly _testDynamic: boolean = false; // True to test dynamic updates (for testing purposes)

// ==================================================
// OVERRIDES
Expand Down Expand Up @@ -98,7 +98,7 @@ class FilterItemCommand extends BaseFilterItemCommand {
return this._color;
}

public setChecked(value: boolean): void {
public override setChecked(value: boolean): void {
if (this._use === value) {
return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,11 @@ import { CommandsUpdater } from '../reactUpdaters/CommandsUpdater';
import { type RevealRenderTarget } from '../renderTarget/RevealRenderTarget';

export class PointCloudFilterCommand extends BaseFilterCommand {
// ==================================================
// INSTANCE FIELDS
// ==================================================

private _modelId: number | undefined = undefined;
private _revisionId: number | undefined = undefined;

// ==================================================
Expand All @@ -21,30 +26,28 @@ export class PointCloudFilterCommand extends BaseFilterCommand {
}

public override get isEnabled(): boolean {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
if (pointCloud === undefined) {
return false;
}
return true;
return this.getPointCloud() !== undefined;
}

public override initializeChildrenIfNeeded(): void {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
if (pointCloud === undefined) {
this._children = undefined;
this._modelId = undefined;
this._revisionId = undefined;
return;
}
if (this._revisionId === pointCloud.revisionId) {
return;
if (this._modelId === pointCloud.modelId && this._revisionId === pointCloud.revisionId) {
return; // Nothing changed
}
this._modelId = pointCloud.modelId;
this._revisionId = pointCloud.revisionId;
this._children = undefined;
super.initializeChildrenIfNeeded();
}

protected createChildren(): FilterItemCommand[] {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
protected override createChildren(): FilterItemCommand[] {
const pointCloud = this.getPointCloud();
if (pointCloud === undefined) {
return [];
}
Expand All @@ -55,25 +58,25 @@ export class PointCloudFilterCommand extends BaseFilterCommand {
const children = [];
for (const c of classes) {
const pointClass = new PointClass(c.name, c.code, c.color);
children.push(new FilterItemCommand(pointClass));
children.push(new FilterItemCommand(pointClass, pointCloud.modelId, pointCloud.revisionId));
}
return children;
}

public override get isAllChecked(): boolean {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
const pointCloud = this.getPointCloud();
if (pointCloud === undefined) {
return false;
}
return isClassesVisible(pointCloud);
return isAllClassesVisible(pointCloud);
}

public override toggleAllChecked(): void {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
const pointCloud = this.getPointCloud();
if (pointCloud === undefined) {
return;
}
const isAllChecked = isClassesVisible(pointCloud);
const isAllChecked = isAllClassesVisible(pointCloud);
const classes = pointCloud.getClasses();
if (classes === undefined || classes.length === 0) {
return;
Expand All @@ -82,19 +85,39 @@ export class PointCloudFilterCommand extends BaseFilterCommand {
pointCloud.setClassVisible(c.code, !isAllChecked);
}
}

// ==================================================
// INSTANCE METHODS
// ==================================================

private getPointCloud(): CognitePointCloudModel | undefined {
if (this._modelId === undefined || this._revisionId === undefined) {
return undefined;
}
for (const pointCloud of this.renderTarget.getPointClouds()) {
if (this._modelId === pointCloud.modelId && this._revisionId === pointCloud.revisionId) {
return pointCloud;
}
}
return undefined;
Comment on lines +97 to +102
Copy link
Contributor

Choose a reason for hiding this comment

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

This function could be rewritten as return this.renderTarget.getPointClouds().find(pointCloud => pointCloud.modelId === this._modelId && pointCloud.revisionId === this._revisionId). It returns undefined if no applicable result is found

}
}

// Note: This is not exported, as it is only used internally

class FilterItemCommand extends BaseFilterItemCommand {
private readonly _modelId: number;
private readonly _revisionId: number;
private readonly _pointClass: PointClass;

// ==================================================
// CONSTRUCTOR
// ==================================================

public constructor(pointClass: PointClass) {
public constructor(pointClass: PointClass, modelId: number, revisionId: number) {
super();
this._modelId = modelId;
this._revisionId = revisionId;
this._pointClass = pointClass;
}

Expand All @@ -107,15 +130,15 @@ class FilterItemCommand extends BaseFilterItemCommand {
}

public override get isChecked(): boolean {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
const pointCloud = this.getPointCloud();
if (pointCloud === undefined) {
return false;
}
return pointCloud.isClassVisible(this._pointClass.code);
}

public override invokeCore(): boolean {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
const pointCloud = this.getPointCloud();
if (pointCloud === undefined) {
return false;
}
Expand All @@ -128,14 +151,27 @@ class FilterItemCommand extends BaseFilterItemCommand {
return this._pointClass.color;
}

public setChecked(value: boolean): void {
const pointCloud = getFirstPointCloudWithClasses(this.renderTarget);
public override setChecked(value: boolean): void {
const pointCloud = this.getPointCloud();
if (pointCloud === undefined) {
return;
}
pointCloud.setClassVisible(this._pointClass.code, value);
CommandsUpdater.update(this._renderTarget);
}

// ==================================================
// INSTANCE METHODS
// ==================================================

private getPointCloud(): CognitePointCloudModel | undefined {
for (const pointCloud of this.renderTarget.getPointClouds()) {
if (this._modelId === pointCloud.modelId && this._revisionId === pointCloud.revisionId) {
return pointCloud;
}
}
return undefined;
Comment on lines +168 to +173
Copy link
Contributor

Choose a reason for hiding this comment

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

This could also be written with find

}
}

class PointClass {
Expand All @@ -161,6 +197,10 @@ class PointClass {
}
}

// ==================================================
// PRIVATE FUNCTIONS
// ==================================================

function getFirstPointCloudWithClasses(
renderTarget: RevealRenderTarget
): CognitePointCloudModel | undefined {
Expand All @@ -169,11 +209,12 @@ function getFirstPointCloudWithClasses(
if (classes === undefined || classes.length === 0) {
continue;
}
return pointCloud;
}
return undefined;
}

function isClassesVisible(pointCloud: CognitePointCloudModel): boolean {
function isAllClassesVisible(pointCloud: CognitePointCloudModel): boolean {
const classes = pointCloud.getClasses();
if (classes === undefined || classes.length === 0) {
return false;
Expand Down
2 changes: 0 additions & 2 deletions react-components/src/architecture/base/undo/Transaction.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ export abstract class Transaction {
throw new Error('Parent is undefined');
}
this.parentUniqueId = parent.uniqueId;
// console.log('Add transaction:', this.changed);
}

// ==================================================
Expand All @@ -52,7 +51,6 @@ export abstract class Transaction {
if (domainObject === undefined) {
return false;
}
// console.log('Undo transaction:', this.changed);
this.undoCore(domainObject, root.renderTarget);
return true;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,9 @@ export class ApplyClipCommand extends RenderTargetCommand {
}

public override get isEnabled(): boolean {
if (this.getSelectedCropBoxDomainObject() !== undefined) {
return true;
const cropBox = this.getSelectedCropBoxDomainObject();
if (cropBox !== undefined) {
return cropBox.focusType !== FocusType.Pending;
}
if (this.rootDomainObject.getDescendantByType(SliceDomainObject) !== undefined) {
return true;
Expand All @@ -56,8 +57,10 @@ export class ApplyClipCommand extends RenderTargetCommand {
}
const cropBox = this.getSelectedCropBoxDomainObject();
if (cropBox !== undefined) {
cropBox.setThisAsGlobalCropBox();
renderTarget.fitView();
if (cropBox.focusType !== FocusType.Pending) {
cropBox.setThisAsGlobalCropBox();
renderTarget.fitView();
}
} else {
ApplyClipCommand.setClippingPlanes(this.rootDomainObject);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ export const FilterButton = ({
[]
);

command.initializeChildrenIfNeeded();

const [isEnabled, setEnabled] = useState<boolean>(true);
const [isVisible, setVisible] = useState<boolean>(true);
const [uniqueId, setUniqueId] = useState<number>(0);
Expand Down Expand Up @@ -94,7 +96,6 @@ export const FilterButton = ({
const shortcut = command.getShortCutKeys();
const flexDirection = getFlexDirection(isHorizontal);

command.initializeChildrenIfNeeded();
const children = command.children;
if (children === undefined || !command.hasChildren) {
return <></>;
Expand Down Expand Up @@ -124,7 +125,7 @@ export const FilterButton = ({
onClick={() => {
command.toggleAllChecked();
}}>
{command.getAllLabel(t)}
{BaseFilterCommand.getAllString(t)}
</Menu.Item>
<StyledMenuItems>
{children.map((child, _index): ReactElement => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,9 @@ const ColorBox = styled.div<{ backgroundColor: Color }>`
`;

const CenteredContainer = styled.div`
padding: 0px 1px;
display: flex;
row-gap: 3px;
gap: 3px;
gap: 4px;
align-items: center;
`;
6 changes: 1 addition & 5 deletions react-components/src/components/Architecture/utilities.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,11 +25,7 @@ export function getTooltipPlacement(isHorizontal: boolean): PlacementType {

export function getButtonType(command: BaseCommand): ButtonType {
// This was the only way it went through compiler: (more button types will be added in the future)
const type = command.buttonType;
if (type === 'ghost' || type === 'ghost-destructive' || type === 'primary') {
return type;
}
return 'ghost';
return command.buttonType as ButtonType;
}

export function getDefaultCommand<T extends BaseCommand>(
Expand Down
Loading