Skip to content

[WEB-2542] Fix: display filter and tooltip issues in list layout. #5696

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
pushya22 merged 3 commits into from
Sep 25, 2024
Merged

[WEB-2542] Fix: display filter and tooltip issues in list layout. #5696

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c0e0d45
[WEB-2542] fix: list layout issues.
prateekshourya29 Sep 25, 2024
f907012
fix: build errors.
prateekshourya29 Sep 25, 2024
73cdbde
fix: lint errors
prateekshourya29 Sep 25, 2024
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
2 changes: 1 addition & 1 deletion web/app/profile/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,8 @@ import { USER_ROLES } from "@/constants/workspace";
import { useUser, useUserSettings } from "@/hooks/store";
// import { ProfileSettingsLayout } from "@/layouts/settings-layout";
// layouts
import { FileService } from "@/services/file.service";
import { ENABLE_LOCAL_DB_CACHE } from "@/plane-web/constants/issues";
import { FileService } from "@/services/file.service";
// services
// types

Expand Down
8 changes: 4 additions & 4 deletions web/core/components/issues/issue-layouts/list/block.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -130,7 +130,7 @@ export const IssueBlock = observer((props: IssueBlockProps) => {
};

//TODO: add better logic. This is to have a min width for ID/Key based on the length of project identifier
const keyMinWidth = (projectIdentifier?.length ?? 0) * 7;
const keyMinWidth = displayProperties?.key ? (projectIdentifier?.length ?? 0) * 7 : 0;

return (
<ControlLink
Expand Down Expand Up @@ -178,7 +178,6 @@ export const IssueBlock = observer((props: IssueBlockProps) => {
</>
}
disabled={issue.project_id === projectId}
renderByDefault={false}
>
<div className="flex-shrink-0 grid place-items-center w-3.5 absolute left-1">
<MultipleSelectEntityAction
Expand All @@ -196,13 +195,14 @@ export const IssueBlock = observer((props: IssueBlockProps) => {
</div>
</Tooltip>
)}
{displayProperties && displayProperties?.key && (
{displayProperties && (displayProperties.key || displayProperties.issue_type) && (
<div className="flex-shrink-0" style={{ minWidth: `${keyMinWidth}px` }}>
{issue.project_id && (
<IssueIdentifier
issueId={issueId}
projectId={issue.project_id}
textContainerClassName="text-xs font-medium text-custom-text-300"
displayProperties={displayProperties}
Comment on lines +198 to +205
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 25, 2024

Choose a reason for hiding this comment

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

💡 Codebase verification

IssueIdentifier does not utilize displayProperties.issue_type

The IssueIdentifier component currently only uses displayProperties.key to determine rendering. The displayProperties.issue_type property is not referenced or utilized within the component. Therefore, the change to include issue_type in the conditional rendering does not have any effect.

  • IssueIdentifier component (web/ce/components/issues/issue-details/issue-identifier.tsx) only references displayProperties.key.
🔗 Analysis chain

Improved flexibility in IssueIdentifier rendering

The modification to render IssueIdentifier when either displayProperties.key or displayProperties.issue_type is present improves the component's flexibility. Passing the entire displayProperties object to IssueIdentifier is a good practice for maintaining consistency.

Please verify that the IssueIdentifier component correctly handles the new displayProperties prop. Run the following script to check its implementation:

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Verify the IssueIdentifier component's handling of displayProperties

# Test: Search for the IssueIdentifier component definition
rg --type typescript -A 10 'export const IssueIdentifier'

Length of output: 94

Script:

#!/bin/bash
# Description: Verify the IssueIdentifier component's handling of displayProperties

# Test: Search for the IssueIdentifier component definition in .ts and .tsx files
rg -g "*.ts" -g "*.tsx" -A 10 'export const IssueIdentifier'

Length of output: 1212

Script:

#!/bin/bash
# Description: Verify the usage of displayProperties within IssueIdentifier component

# Test: Search for usage of displayProperties in issue-identifier.tsx
rg -g 'issue-identifier.tsx' 'displayProperties'

Length of output: 439

Script:

#!/bin/bash
# Description: Verify all usages of displayProperties within IssueIdentifier component

# Test: Search for specific properties of displayProperties in issue-identifier.tsx
rg -g 'issue-identifier.tsx' 'displayProperties\.(key|issue_type)'

Length of output: 206

/>
)}
</div>
Expand Down Expand Up @@ -293,4 +293,4 @@ export const IssueBlock = observer((props: IssueBlockProps) => {
</Row>
</ControlLink>
);
});
});
4 changes: 2 additions & 2 deletions web/core/layouts/auth-layout/project-wrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,6 @@ import { FC, ReactNode } from "react";
import { observer } from "mobx-react";
import { useParams } from "next/navigation";
import useSWR from "swr";
import useSWRImmutable from "swr/immutable";

// components
import { JoinProject } from "@/components/auth-screens";
Expand All @@ -23,10 +22,11 @@ import {
useProjectView,
useUserPermissions,
} from "@/hooks/store";
// local
import { persistence } from "@/local-db/storage.sqlite";
// plane web constants
import { EUserPermissions, EUserPermissionsLevel } from "@/plane-web/constants/user-permissions";
// images
import { persistence } from "@/local-db/storage.sqlite";
import emptyProject from "@/public/empty-state/onboarding/dashboard-light.webp";

interface IProjectAuthWrapper {
Expand Down
3 changes: 2 additions & 1 deletion web/core/layouts/auth-layout/workspace-wrapper.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@ import { LogoSpinner } from "@/components/common";
import { useMember, useProject, useUser, useUserPermissions, useWorkspace } from "@/hooks/store";
import { useFavorite } from "@/hooks/store/use-favorite";
import { usePlatformOS } from "@/hooks/use-platform-os";
// local
import { persistence } from "@/local-db/storage.sqlite";
// constants
import { EUserPermissions, EUserPermissionsLevel } from "@/plane-web/constants/user-permissions";
// images
import { persistence } from "@/local-db/storage.sqlite";
import PlaneBlackLogo from "@/public/plane-logos/black-horizontal-with-blue-logo.png";
import PlaneWhiteLogo from "@/public/plane-logos/white-horizontal-with-blue-logo.png";
import WorkSpaceNotAvailable from "@/public/workspace/workspace-not-available.png";
Expand Down
2 changes: 1 addition & 1 deletion web/core/lib/polyfills/requestIdleCallback.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ if (typeof window !== "undefined" && window) {
window.requestIdleCallback =
window.requestIdleCallback ??
function (cb) {
var start = Date.now();
const start = Date.now();
return setTimeout(function () {
cb({
didTimeout: false,
Expand Down
2 changes: 1 addition & 1 deletion web/core/local-db/storage.sqlite.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ export class Storage {
try {
const storageManager = window.navigator.storage;
const fileSystemDirectoryHandle = await storageManager.getDirectory();
//@ts-expect-error
//@ts-expect-error , clear local issue cache
await fileSystemDirectoryHandle.remove({ recursive: true });
} catch (e) {
console.error("Error clearing sqlite sync storage", e);
Expand Down
4 changes: 1 addition & 3 deletions web/core/local-db/utils/query.utils.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,9 +77,7 @@ export const getMetaKeysFragment = (queries: any) => {
}
});

let sql;

sql = ` ('${Array.from(fields).join("','")}')`;
const sql = ` ('${Array.from(fields).join("','")}')`;

return sql;
};
Expand Down
1 change: 1 addition & 0 deletions web/core/store/project/project.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -408,6 +408,7 @@ export class ProjectStore implements IProjectStore {
} catch (error) {
console.log("Failed to delete project from project store");
this.fetchProjects(workspaceSlug);
throw error;
}
};

Expand Down
2 changes: 1 addition & 1 deletion web/core/store/user/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import { TUserPermissions } from "@plane/types/src/enums";
import { API_BASE_URL } from "@/helpers/common.helper";
// local
import { persistence } from "@/local-db/storage.sqlite";
import { ENABLE_LOCAL_DB_CACHE } from "@/plane-web/constants/issues";
import { EUserPermissions } from "@/plane-web/constants/user-permissions";
// services
import { AuthService } from "@/services/auth.service";
Expand All @@ -19,7 +20,6 @@ import { IAccountStore } from "@/store/user/account.store";
import { ProfileStore, IUserProfileStore } from "@/store/user/profile.store";
import { IUserPermissionStore, UserPermissionStore } from "./permissions.store";
import { IUserSettingsStore, UserSettingsStore } from "./settings.store";
import { ENABLE_LOCAL_DB_CACHE } from "@/plane-web/constants/issues";

type TUserErrorStatus = {
status: string;
Expand Down
8 changes: 4 additions & 4 deletions web/core/store/user/profile.store.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -81,11 +81,11 @@ export class ProfileStore implements IUserProfileStore {

// helper action
mutateUserProfile = (data: Partial<TUserProfile>) => {
if (!data) return
if (!data) return;
Object.entries(data).forEach(([key, value]) => {
if (key in this.data) set(this.data, key, value);
})
}
});
};

// actions
/**
Expand Down Expand Up @@ -129,7 +129,7 @@ export class ProfileStore implements IUserProfileStore {
}
const userProfile = await this.userService.updateCurrentUserProfile(data);
return userProfile;
} catch (error) {
} catch {
Copy link
Contributor

@coderabbitai coderabbitai bot Sep 25, 2024

Choose a reason for hiding this comment

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

⚠️ Potential issue

Consider keeping the error parameter for debugging.

While the current implementation still updates the error state with a generic message, removing the error parameter from the catch block means that specific error information is lost. This could make it more difficult to diagnose issues if they occur.

Consider keeping the error parameter and logging it for debugging purposes. Here's a suggested change:

-    } catch {
+    } catch (error) {
+      console.error("Error updating user profile:", error);
       if (currentUserProfileData) {
         this.mutateUserProfile(currentUserProfileData);
       }

This way, you maintain the generic user-facing error message while still having access to the specific error details for debugging.

📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
} catch {
} catch (error) {
console.error("Error updating user profile:", error);

if (currentUserProfileData) {
this.mutateUserProfile(currentUserProfileData);
}
Expand Down