Skip to content
Closed
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
82 changes: 70 additions & 12 deletions src/components/ResultsView.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -314,8 +314,13 @@ const ResultsView: ResultsViewComponent = ({
const [columnFilters, setColumnFilters] = useState(settings.columnFilters);
const [searchFilter, setSearchFilter] = useState(settings.searchFilter);
const [showInterface, setShowInterface] = useState(settings.showInterface);
const [hiddenColumns, setHiddenColumns] = useState(settings.hiddenColumns);
const [showMenuIcons, setShowMenuIcons] = useState(false);

const visibleColumns = useMemo(() => {
return columns.filter(column => !hiddenColumns.includes(column.key));
}, [columns, hiddenColumns]);

const { allProcessedResults, paginatedResults } = useMemo(() => {
return postProcessResults(results, {
activeSort,
Expand Down Expand Up @@ -360,6 +365,7 @@ const ResultsView: ResultsViewComponent = ({
const [isEditColumnSort, setIsEditColumnSort] = useState(false);
const [isEditColumnFilter, setIsEditColumnFilter] = useState(false);
const [isEditSearchFilter, setIsEditSearchFilter] = useState(false);
const [isEditHiddenColumns, setIsEditHiddenColumns] = useState(false);

const [layout, setLayout] = useState(settings.layout);
const layoutMode = useMemo(
Expand All @@ -371,8 +377,9 @@ const ResultsView: ResultsViewComponent = ({
searchFilter ||
columnFilters.length ||
random.count ||
hiddenColumns.length ||
(activeSort.length && layout.mode !== "table"), // indicator is on ResultHeader
[searchFilter, columnFilters, random, activeSort, layout.mode]
[searchFilter, columnFilters, random, activeSort, layout.mode, hiddenColumns]
);
const onViewChange = (view: (typeof views)[number], i: number) => {
const newViews = views.map((v, j) => (i === j ? view : v));
Expand Down Expand Up @@ -405,6 +412,23 @@ const ResultsView: ResultsViewComponent = ({
(view) => VIEWS[view.mode]?.value === true
);

const onHiddenColumnsChange = (newHiddenColumns: string[]) => {
setHiddenColumns(newHiddenColumns);
if (preventSavingSettings) return;
const hiddenColumnsNode = getSubTree({
key: "hiddenColumns",
parentUid: settings.resultNodeUid,
});
hiddenColumnsNode.children.forEach((c) => deleteBlock(c.uid));
newHiddenColumns.forEach((columnKey, order) =>
createBlock({
parentUid: hiddenColumnsNode.uid,
node: { text: columnKey },
order,
})
);
};

return (
<div
className={`roamjs-query-results-view w-full relative mode-${layout.mode}`}
Expand Down Expand Up @@ -457,7 +481,7 @@ const ResultsView: ResultsViewComponent = ({
isOpen={isExportOpen}
onClose={handleCloseExport}
results={allProcessedResults}
columns={columns}
columns={visibleColumns}
/>
<div className="relative">
<div
Expand All @@ -476,6 +500,7 @@ const ResultsView: ResultsViewComponent = ({
setIsEditColumnFilter(false);
setIsEditViews(false);
setIsEditColumnSort(false);
setIsEditHiddenColumns(false);
}}
autoFocus={false}
enforceFocus={false}
Expand Down Expand Up @@ -575,7 +600,7 @@ const ResultsView: ResultsViewComponent = ({
{settingsById[layoutMode].map((s) => {
const options =
s.options === "columns"
? columns.map((c) => c.key)
? visibleColumns.map((c) => c.key)
: s.options.slice();
return (
<Label key={s.key}>
Expand Down Expand Up @@ -654,7 +679,7 @@ const ResultsView: ResultsViewComponent = ({
</div>
))}
<MenuItemSelect
items={columns
items={visibleColumns
.map((c) => c.key)
.filter((c) => !activeSort.some((as) => as.key === c))}
transformItem={(k) =>
Expand All @@ -663,7 +688,7 @@ const ResultsView: ResultsViewComponent = ({
ButtonProps={{
text: "Choose Column",
intent: "primary",
disabled: columns.length === activeSort.length,
disabled: visibleColumns.length === activeSort.length,
}}
onItemSelect={(column) => {
resultViewSetActiveSort([
Expand All @@ -689,7 +714,7 @@ const ResultsView: ResultsViewComponent = ({
<div className="flex items-center justify-between gap-2 mb-2">
<MenuItemSelect
className="roamjs-column-filter-key"
items={columns.map((c) => c.key)}
items={visibleColumns.map((c) => c.key)}
transformItem={(k) =>
k.length > 10 ? `${k.slice(0, 7)}...` : k
}
Expand Down Expand Up @@ -825,7 +850,7 @@ const ResultsView: ResultsViewComponent = ({
intent="primary"
onClick={() => {
const newFilter = {
key: columns[0].key,
key: visibleColumns[0].key,
type: SUPPORTED_COLUMN_FILTER_TYPES[0].id,
value: [""],
uid: window.roamAlphaAPI.util.generateUID(),
Expand Down Expand Up @@ -884,7 +909,7 @@ const ResultsView: ResultsViewComponent = ({
</tr>
</thead>
<tbody className="bg-white divide-y divide-gray-200">
{views.map(({ column, mode, value }, i) => (
{views.filter(({ column }) => visibleColumns.some(c => c.key === column)).map(({ column, mode, value }, i) => (
<tr key={i}>
<td className="whitespace-nowrap">{column}</td>
<td className="whitespace-nowrap">
Expand Down Expand Up @@ -942,6 +967,31 @@ const ResultsView: ResultsViewComponent = ({
</tbody>
</HTMLTable>
</div>
) : isEditHiddenColumns ? (
<div className="relative p-4" style={{ minWidth: "320px" }}>
<MenuHeading
onClear={() => setIsEditHiddenColumns(false)}
text="Hide Columns"
/>
<div className="flex flex-col gap-2 py-2">
{columns.map(({ key }) => (
<div key={key} className="flex items-center justify-between">
<span className="text-sm">{key}</span>
<Switch
checked={!hiddenColumns.includes(key)}
onChange={(e) => {
const isVisible = (e.target as HTMLInputElement).checked;
if (isVisible) {
onHiddenColumnsChange(hiddenColumns.filter(col => col !== key));
} else {
onHiddenColumnsChange([...hiddenColumns, key]);
}
}}
/>
</div>
))}
</div>
</div>
Comment on lines +970 to +994
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Consider preventing users from hiding all columns

The implementation looks good, but there's an edge case where users could potentially hide all columns, leaving the results view empty. Consider adding validation to ensure at least one column remains visible.

 onChange={(e) => {
   const isVisible = (e.target as HTMLInputElement).checked;
   if (isVisible) {
     onHiddenColumnsChange(hiddenColumns.filter(col => col !== key));
   } else {
+    // Prevent hiding the last visible column
+    if (hiddenColumns.length >= columns.length - 1) {
+      renderToast({
+        id: "hide-columns-error",
+        content: "At least one column must remain visible",
+        intent: Intent.WARNING,
+      });
+      return;
+    }
     onHiddenColumnsChange([...hiddenColumns, key]);
   }
 }}
📝 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
) : isEditHiddenColumns ? (
<div className="relative p-4" style={{ minWidth: "320px" }}>
<MenuHeading
onClear={() => setIsEditHiddenColumns(false)}
text="Hide Columns"
/>
<div className="flex flex-col gap-2 py-2">
{columns.map(({ key }) => (
<div key={key} className="flex items-center justify-between">
<span className="text-sm">{key}</span>
<Switch
checked={!hiddenColumns.includes(key)}
onChange={(e) => {
const isVisible = (e.target as HTMLInputElement).checked;
if (isVisible) {
onHiddenColumnsChange(hiddenColumns.filter(col => col !== key));
} else {
onHiddenColumnsChange([...hiddenColumns, key]);
}
}}
/>
</div>
))}
</div>
</div>
) : isEditHiddenColumns ? (
<div className="relative p-4" style={{ minWidth: "320px" }}>
<MenuHeading
onClear={() => setIsEditHiddenColumns(false)}
text="Hide Columns"
/>
<div className="flex flex-col gap-2 py-2">
{columns.map(({ key }) => (
<div key={key} className="flex items-center justify-between">
<span className="text-sm">{key}</span>
<Switch
checked={!hiddenColumns.includes(key)}
onChange={(e) => {
const isVisible = (e.target as HTMLInputElement).checked;
if (isVisible) {
onHiddenColumnsChange(hiddenColumns.filter(col => col !== key));
} else {
// Prevent hiding the last visible column
if (hiddenColumns.length >= columns.length - 1) {
renderToast({
id: "hide-columns-error",
content: "At least one column must remain visible",
intent: Intent.WARNING,
});
return;
}
onHiddenColumnsChange([...hiddenColumns, key]);
}
}}
/>
</div>
))}
</div>
</div>
🤖 Prompt for AI Agents
In src/components/ResultsView.tsx around lines 970 to 994, the current code
allows users to hide all columns, which can result in an empty results view. To
fix this, add validation in the onChange handler of the Switch component to
prevent hiding the last visible column. Specifically, check if hiding the
current column would leave no visible columns, and if so, do not update
hiddenColumns state to ensure at least one column remains visible.

) : (
<Menu>
{onEdit && (
Expand Down Expand Up @@ -975,6 +1025,14 @@ const ResultsView: ResultsViewComponent = ({
setIsEditViews(true);
}}
/>
<MenuItem
icon={"eye-off"}
text={"Hide Columns"}
className={hiddenColumns.length ? "roamjs-item-dirty" : ""}
onClick={() => {
setIsEditHiddenColumns(true);
}}
/>
<MenuItem
icon={isEditSearchFilter ? "zoom-out" : "search"}
text={isEditSearchFilter ? "Hide Search" : "Search"}
Expand Down Expand Up @@ -1130,7 +1188,7 @@ const ResultsView: ResultsViewComponent = ({
layoutMode === "table" ? (
<ResultsTable
layout={layout}
columns={columns}
columns={visibleColumns}
results={paginatedResults}
parentUid={settings.resultNodeUid}
activeSort={activeSort}
Expand All @@ -1152,13 +1210,13 @@ const ResultsView: ResultsViewComponent = ({
<Charts
type="line"
data={allProcessedResults}
columns={columns.slice(1)}
columns={visibleColumns.slice(1)}
/>
) : layoutMode === "bar" ? (
<Charts
type="bar"
data={allProcessedResults}
columns={columns.slice(1)}
columns={visibleColumns.slice(1)}
/>
) : layoutMode === "timeline" ? (
<Timeline timelineElements={allProcessedResults} />
Expand All @@ -1167,7 +1225,7 @@ const ResultsView: ResultsViewComponent = ({
data={allProcessedResults}
layout={layout}
onQuery={() => onRefresh(true)}
resultKeys={columns}
resultKeys={visibleColumns}
parentUid={parentUid}
views={views}
activeSort={activeSort}
Expand Down
5 changes: 5 additions & 0 deletions src/utils/parseResultSettings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,10 @@ const parseResultSettings = (
tree: resultNode.children,
key: "layout",
});
const hiddenColumnsNode = getSubTree({
tree: resultNode.children,
key: "hiddenColumns",
});
const layout = Object.fromEntries(
layoutNode.children
.filter((c) => c.children.length)
Expand Down Expand Up @@ -161,6 +165,7 @@ const parseResultSettings = (
savedViewData[column]?.mode || (column === "text" ? "link" : "plain"),
value: savedViewData[column]?.value || "",
})),
hiddenColumns: hiddenColumnsNode.children.map((c) => c.text),
random,
pageSize,
layout,
Expand Down