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: OPTIC-1178: Possible LS Memory Leak #6704

Merged
merged 26 commits into from
Dec 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
eedfa36
fix: OPTIC-1178: Possible LS Memory Leak
yyassi-heartex Nov 22, 2024
18890b6
lint cleanup
yyassi-heartex Nov 22, 2024
806e5db
low hanging fruit error cleanup
yyassi-heartex Nov 22, 2024
753f6e1
Merge branch 'develop' into fb-optic-1178/memory-leak
yyassi-heartex Nov 25, 2024
47f3a60
changing how we handle images + content-visibility
yyassi-heartex Nov 25, 2024
312ba13
Merge branch 'develop' into fb-optic-1178/memory-leak
yyassi-heartex Nov 26, 2024
9b737b9
leaning more on to background image over image + cleaning console out…
yyassi-heartex Nov 26, 2024
b553151
linting + adding testid to div
yyassi-heartex Nov 26, 2024
cc54c87
moving way from block and elem
yyassi-heartex Nov 26, 2024
288d456
lint clean up
yyassi-heartex Nov 26, 2024
eb020dd
replacing Block and Elem with div and span
yyassi-heartex Nov 27, 2024
60594f8
reducing Block and Elem usage
yyassi-heartex Nov 27, 2024
2382036
reducing Block and Elem usage
yyassi-heartex Nov 27, 2024
e9f44db
undoing testing code
yyassi-heartex Nov 27, 2024
05707c5
eleminating long standing console error
yyassi-heartex Nov 27, 2024
bc6f813
Merge branch 'develop' into fb-optic-1178/memory-leak
yyassi-heartex Dec 2, 2024
e3bb5c9
lint cleanup
yyassi-heartex Dec 2, 2024
2f77a0b
Update web/libs/datamanager/src/components/MainView/GridView/GridView…
yyassi-heartex Dec 2, 2024
d03618b
Merge branch 'develop' into fb-optic-1178/memory-leak
yyassi-heartex Dec 3, 2024
5f84b12
Merge branch 'develop' into 'fb-optic-1178/memory-leak'
yyassi-heartex Dec 12, 2024
71669af
Merge branch 'develop' into 'fb-optic-1178/memory-leak'
yyassi-heartex Dec 13, 2024
86bc33e
Merge branch 'develop' into 'fb-optic-1178/memory-leak'
yyassi-heartex Dec 13, 2024
b9aaa19
clean up console
yyassi-heartex Dec 13, 2024
9874aac
Merge branch 'fb-optic-1178/memory-leak' of github.com:heartexlabs/la…
yyassi-heartex Dec 13, 2024
6f14092
undoing the img to div with backgorund to avoid cors issues detected …
yyassi-heartex Dec 16, 2024
82c9232
Merge branch 'develop' into 'fb-optic-1178/memory-leak'
yyassi-heartex Dec 16, 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
26 changes: 16 additions & 10 deletions web/libs/datamanager/src/components/App/App.jsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { observer, Provider } from "mobx-react";
import React from "react";
import clsx from "clsx";
import { SDKProvider } from "../../providers/SDKProvider";
import { Block, Elem } from "../../utils/bem";
import { cn } from "../../utils/bem";
import { Spinner } from "../Common/Spinner";
import { DataManager } from "../DataManager/DataManager";
import { Labeling } from "../Label/Label";
Expand All @@ -26,27 +27,32 @@ class ErrorBoundary extends React.Component {
* @param {{app: import("../../stores/AppStore").AppStore} param0
*/
const AppComponent = ({ app }) => {
const rootCN = cn("root");
const rootClassName = rootCN.mod({ mode: app.SDK.mode }).toString();
const crashCN = cn("crash");
return (
<ErrorBoundary>
<Provider store={app}>
<SDKProvider sdk={app.SDK}>
<Block name="root" mod={{ mode: app.SDK.mode }}>
<div className={rootClassName}>
{app.crashed ? (
<Block name="crash">
<Elem name="header">Oops...</Elem>
<Elem name="description">Project has been deleted or not yet created.</Elem>
</Block>
<div className={clsx(rootCN.toString(), rootClassName)}>
<span className={rootCN.elem("header").toString()}>Oops...</span>
<span className={rootCN.elem("description").toString()}>
Project has been deleted or not yet created.
</span>
</div>
) : app.loading ? (
<Block name="app-loader">
<div className={cn("app-loader").toString()}>
<Spinner size="large" />
</Block>
</div>
) : app.isLabeling ? (
<Labeling />
) : (
<DataManager />
)}
<Block name={"offscreen"} />
</Block>
<div className={cn("offscreen").toString()} />
</div>
</SDKProvider>
</Provider>
</ErrorBoundary>
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { Block, Elem } from "../../../utils/bem";
import clsx from "clsx";
import { cn } from "../../../utils/bem";
import { isDefined } from "../../../utils/utils";
import "./Agreement.scss";

Expand All @@ -19,12 +20,14 @@ const formatNumber = (num) => {
};

export const Agreement = (column) => {
const agreementCN = cn("agreement");
const scoreElem = agreementCN.elem("score");
return (
<Block name="agreement">
<Elem name="score" mod={{ [agreement(column.value)]: true }}>
<div className={agreementCN.toString()}>
<span className={clsx(scoreElem.toString(), scoreElem.mod({ [agreement(column.value)]: true }).toString())}>
{isDefined(column.value) ? `${formatNumber(column.value)}%` : ""}
</Elem>
</Block>
</span>
</div>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,8 @@
import { inject } from "mobx-react";
import clsx from "clsx";
import { LsCheckAlt, LsCrossAlt } from "../../../assets/icons";
import { useSDK } from "../../../providers/SDKProvider";
import { Block, Elem } from "../../../utils/bem";
import { cn } from "../../../utils/bem";
import { isDefined } from "../../../utils/utils";
import { Space } from "../../Common/Space/Space";
import { Tooltip } from "../../Common/Tooltip/Tooltip";
Expand All @@ -16,10 +17,12 @@ export const Annotators = (cell) => {
const userList = Array.from(value);
const renderable = userList.slice(0, 10);
const extra = userList.length - renderable.length;
const userPickBadge = cn("userpic-badge");
const annotatorsCN = cn("annotators");

return (
<Block name="annotators">
{renderable.map((item) => {
<div className={annotatorsCN.toString()}>
{renderable.map((item, index) => {
const user = item.user ?? item;
const { annotated, reviewed, review } = item;

Expand All @@ -28,9 +31,9 @@ export const Annotators = (cell) => {
const suppressStats = column.alias === "comment_authors";

return (
<Elem
key={`user-${user.id}`}
name="item"
<div
key={`user-${user.id}-${index}`}
className={annotatorsCN.elem("item").toString()}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
Expand All @@ -43,29 +46,29 @@ export const Annotators = (cell) => {
faded={userpicIsFaded}
badge={{
bottomRight: review && (
<Block name="userpic-badge" mod={{ [review]: true }}>
<div className={clsx(userPickBadge.toString(), userPickBadge.mod({ [review]: true }).toString())}>
{review === "rejected" ? <LsCrossAlt /> : <LsCheckAlt />}
</Block>
</div>
),
}}
/>
</Tooltip>
</Elem>
</div>
);
})}
{extra > 0 && (
<Elem
name="item"
<div
className={annotatorsCN.elem("item").toString()}
onClick={(e) => {
e.preventDefault();
e.stopPropagation();
sdk.invoke("userCellCounterClick", e, column.alias, task, userList);
}}
>
<Userpic username={`+${extra}`} />
</Elem>
</div>
)}
</Block>
</div>
);
};

Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import { inject, observer } from "mobx-react";
import React from "react";
import { taskToLSFormat } from "../../../sdk/lsf-utils";
import { Block } from "../../../utils/bem";
import { cn } from "../../../utils/bem";
import { FF_LSDV_4711, isFF } from "../../../utils/feature-flags";
import { Spinner } from "../Spinner";
import "./AnnotationPreview.scss";
Expand Down Expand Up @@ -135,7 +135,7 @@ export const AnnotationPreview = injector(
height={props.height}
/>
) : (
<Block name="annotation-preview" width={props.width} height={props.height}>
<div className={cn("annotation-preview").toString()} width={props.width} height={props.height}>
<Spinner
size={props.size ?? "default"}
style={{
Expand All @@ -153,7 +153,7 @@ export const AnnotationPreview = injector(
width={props.width}
height={props.height}
/>
</Block>
</div>
);
}),
);
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import React from "react";
import ReactDOM from "react-dom";
import { Block, cn } from "../../../utils/bem";
import clsx from "clsx";
import { cn } from "../../../utils/bem";
import { alignElements } from "../../../utils/dom";
import { aroundTransition } from "../../../utils/transition";
import "./Dropdown.scss";
Expand Down Expand Up @@ -151,17 +152,15 @@ export const Dropdown = React.forwardRef(({ animated = true, visible = false, ..
...(offset ?? {}),
zIndex: 1000 + dropdownIndex,
};

const result = (
<Block
<div
ref={dropdown}
name="dropdown-dm"
mix={[props.className, visibilityClasses]}
className={clsx(rootName.toString(), rootName.mix([props.className, visibilityClasses]).toString())}
style={compositeStyles}
onClick={(e) => e.stopPropagation()}
>
{content}
</Block>
</div>
);

return props.inline === true ? result : ReactDOM.createPortal(result, document.body);
Expand Down
3 changes: 2 additions & 1 deletion web/libs/datamanager/src/components/Common/Form/Form.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -474,8 +474,9 @@ Form.Builder = forwardRef(
};

const renderColumns = (columns) => {
const columnClassName = cn("form-dm").elem("column").toString();
return columns.map((col, index) => (
<div className={cn("form-dm").elem("column")} key={index} style={{ width: col.width }}>
<div className={columnClassName} key={index} style={{ width: col.width }}>
{renderFields(col.fields)}
</div>
));
Expand Down
6 changes: 3 additions & 3 deletions web/libs/datamanager/src/components/Common/Icon/Icon.jsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
import React from "react";
import { Block } from "../../../utils/bem";
import { cn } from "../../../utils/bem";
import "./Icon.scss";

export const Icon = React.forwardRef(({ icon, ...props }, ref) => {
return (
<Block tag="span" name="icon" ref={ref}>
<span className={cn("icon").toString()} ref={ref}>
{React.createElement(icon, props)}
</Block>
</span>
);
});
16 changes: 8 additions & 8 deletions web/libs/datamanager/src/components/Common/Menu/Menu.jsx
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
import React, { useCallback, useMemo } from "react";
import { Block, cn } from "../../../utils/bem";
import clsx from "clsx";
import { cn } from "../../../utils/bem";
import { useDropdown } from "../Dropdown/DropdownTrigger";
import "./Menu.scss";
import { MenuContext } from "./MenuContext";
import { MenuItem } from "./MenuItem";

const menuCN = cn("menu-dm");

export const Menu = React.forwardRef(
({ children, className, style, size, selectedKeys, closeDropdownOnItemClick }, ref) => {
const dropdown = useDropdown();
Expand All @@ -15,7 +18,7 @@ export const Menu = React.forwardRef(

const clickHandler = useCallback(
(e) => {
const elem = cn("menu-dm").elem("item").closest(e.target);
const elem = menuCN.elem("item").closest(e.target);

if (dropdown && elem && closeDropdownOnItemClick !== false) {
dropdown.close();
Expand All @@ -30,17 +33,14 @@ export const Menu = React.forwardRef(

return (
<MenuContext.Provider value={{ selected }}>
<Block
<ul
ref={ref}
tag="ul"
name="menu-dm"
mod={{ size, collapsed }}
mix={className}
className={clsx(menuCN.toString(), menuCN.mod({ size, collapsed }).toString(), className)}
style={style}
onClick={clickHandler}
>
{children}
</Block>
</ul>
</MenuContext.Provider>
);
},
Expand Down
33 changes: 16 additions & 17 deletions web/libs/datamanager/src/components/Common/Table/Table.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -13,11 +13,11 @@ import { modal } from "../Modal/Modal";
import { Tooltip } from "../Tooltip/Tooltip";
import "./Table.scss";
import { TableCheckboxCell } from "./TableCheckbox";
import { TableBlock, TableContext, TableElem } from "./TableContext";
import { tableCN, TableContext } from "./TableContext";
import { TableHead } from "./TableHead/TableHead";
import { TableRow } from "./TableRow/TableRow";
import { prepareColumns } from "./utils";
import { Block } from "../../../utils/bem";
import { cn } from "../../../utils/bem";
import { FieldsButton } from "../FieldsButton";
import { LsGear, LsGearNewUI } from "../../../assets/icons";
import { FF_DEV_3873, FF_LOPS_E_10, FF_LOPS_E_3, isFF } from "../../../utils/feature-flags";
Expand Down Expand Up @@ -285,18 +285,19 @@ export const Table = observer(
tableWrapper.current?.firstChild?.firstChild.offsetWidth -
tableWrapper.current?.firstChild?.firstChild?.firstChild.offsetWidth || 0;

const columnsSelectorCN = cn("columns__selector");
return (
<>
{view.root.isLabeling && (
<Block
name="columns__selector"
<div
className={columnsSelectorCN.toString()}
style={{
right,
}}
>
{isFF(FF_DEV_3873) ? (
<FieldsButton
className={"columns__selector__button-new"}
className={columnsSelectorCN.elem("button-new").toString()}
wrapper={FieldsButton.Checkbox}
icon={<LsGearNewUI />}
style={{ padding: "0" }}
Expand All @@ -316,9 +317,9 @@ export const Table = observer(
}}
/>
)}
</Block>
</div>
)}
<TableBlock ref={tableWrapper} name="table" mod={{ fit: props.fitToContent }}>
<div ref={tableWrapper} className={tableCN.mod({ fit: props.fitToContent }).toString()}>
<TableContext.Provider value={contextValue}>
<StickyList
ref={listRef}
Expand All @@ -338,7 +339,7 @@ export const Table = observer(
{renderRow}
</StickyList>
</TableContext.Provider>
</TableBlock>
</div>
</>
);
},
Expand Down Expand Up @@ -388,7 +389,7 @@ const StickyList = observer(

return (
<StickyListContext.Provider value={itemData}>
<TableElem tag={AutoSizer} name="auto-size">
<AutoSizer className={tableCN.elem("auto-size")}>
{({ width, height }) => (
<InfiniteLoader
ref={listRef}
Expand All @@ -399,9 +400,8 @@ const StickyList = observer(
minimumBatchSize={30}
>
{({ onItemsRendered, ref }) => (
<TableElem
name="virual"
tag={VariableSizeList}
<VariableSizeList
className={tableCN.elem("virual").toString()}
{...rest}
ref={ref}
width={width}
Expand All @@ -412,11 +412,11 @@ const StickyList = observer(
initialScrollOffset={initialScrollOffset?.(height) ?? 0}
>
{ItemWrapper}
</TableElem>
</VariableSizeList>
)}
</InfiniteLoader>
)}
</TableElem>
</AutoSizer>
</StickyListContext.Provider>
);
}),
Expand All @@ -430,9 +430,8 @@ const innerElementType = forwardRef(({ children, ...rest }, ref) => {
{({ stickyItems, stickyItemsHeight, StickyComponent }) => (
<div ref={ref} {...rest}>
{stickyItems.map((index) => (
<TableElem
name="sticky-header"
tag={StickyComponent}
<StickyComponent
className={tableCN.elem("sticky-header").toString()}
key={index}
index={index}
style={{
Expand Down
Loading
Loading