Skip to content

fix(manager-dashboard): correctly set isPano for street projects #988

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.

Sign up for GitHub

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

Merged
ofr1tz merged 1 commit into from
Jan 30, 2025
Merged

fix(manager-dashboard): correctly set isPano for street projects #988

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
7 changes: 4 additions & 3 deletions manager-dashboard/app/views/NewProject/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ const defaultProjectFormValue: PartialProjectFormType = {
// maxTasksPerUser: -1,
inputType: PROJECT_INPUT_TYPE_UPLOAD,
filter: FILTER_BUILDINGS,
isPano: false,
panoOnly: false,
};

interface Props {
Expand Down Expand Up @@ -320,6 +320,7 @@ function NewProject(props: Props) {

valuesToCopy.startTimestamp = valuesToCopy.dateRange?.startDate ?? null;
valuesToCopy.endTimestamp = valuesToCopy.dateRange?.endDate ?? null;
valuesToCopy.isPano = valuesToCopy.panoOnly ? true : null;
Copy link
Collaborator

@Gigaszi Gigaszi Jan 27, 2025

Choose a reason for hiding this comment

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

so it can not be false. or? Is this intended?

Copy link
Collaborator Author

@ofr1tz ofr1tz Jan 27, 2025

Choose a reason for hiding this comment

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

This is as originally intended. The input allows a project creator to decide whether they want all images (isPano = null) or only panoramic ones (isPano = true). In case there should be a need for creating projects with only non-panoramic features, we could adapt the Manager Dashboard later accordingly.


const storage = getStorage();
const timestamp = (new Date()).getTime();
Expand Down Expand Up @@ -756,8 +757,8 @@ function NewProject(props: Props) {
disabled={submissionPending || projectTypeEmpty}
/>
<Checkbox
name={'isPano' as const}
value={value?.isPano}
name={'panoOnly' as const}
value={value?.panoOnly}
label="Only use 360 degree panorama images."
onChange={setFieldValue}
disabled={submissionPending || projectTypeEmpty}
Expand Down
6 changes: 5 additions & 1 deletion manager-dashboard/app/views/NewProject/utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,8 @@ export interface ProjectFormType {
endTimestamp?: string | null;
organizationId?: number;
creatorId?: number;
isPano?: boolean;
panoOnly?: boolean;
isPano?: boolean | null;
Copy link
Collaborator

@Gigaszi Gigaszi Jan 27, 2025

Choose a reason for hiding this comment

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

shouldnt isPano be removed here?

Copy link
Collaborator Author

@ofr1tz ofr1tz Jan 27, 2025

Choose a reason for hiding this comment

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

I think this needs to remain as a type check for valuesToCopy.isPano.

samplingThreshold?: number;
}

Expand Down Expand Up @@ -305,6 +306,9 @@ export const projectFormSchema: ProjectFormSchema = {
greaterThanCondition(0),
],
},
panoOnly: {
required: false,
},
isPano: {
required: false,
},
Expand Down
Loading