-
-
Notifications
You must be signed in to change notification settings - Fork 1.4k
feat: Add button to view table to open all pointers of a column in new browser tabs #2976
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
feat: Add button to view table to open all pointers of a column in new browser tabs #2976
Conversation
🚀 Thanks for opening this pull request! We appreciate your effort in improving the project. Please let us know once your pull request is ready for review. |
📝 WalkthroughWalkthroughAdds pointer-aware column typing, renders a pointer action icon in column headers, and implements Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant U as User
participant V as Views Component
participant D as Data Rows
participant P as Path Generator
participant B as Browser
participant N as Notification
rect rgba(230,240,255,0.5)
Note over V: Render headers (Pointer columns show icon)
U->>V: Click pointer icon (columnName)
V->>D: Collect values for columnName
D-->>V: Values (may include Pointers)
V->>V: Deduplicate by className-objectId
end
alt Any pointers found
loop For each unique pointer
V->>P: generatePath(className, filter on objectId)
P-->>V: URL
V->>B: window.open(URL, "_blank", "noopener,noreferrer")
B-->>V: Opened window or blocked
end
alt All opened
V->>N: showNote(success)
else Some blocked
V->>N: showNote(partial)
end
else None found
V->>N: showNote(none)
end
Note over V,B: Each open wrapped in try/catch with console.error on failure
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal). Please share your feedback with us on this Discord post. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
🎉 Snyk checks have passed. No issues have been found so far.✅ security/snyk check is complete. No issues have been found. (View Details) |
Uffizzi Ephemeral Environment
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 3
🧹 Nitpick comments (1)
src/dashboard/Data/Views/Views.react.js (1)
862-913
: Batch-open guardrails: confirm on large counts, safer dedup key, and noopener
- Ask for confirmation when opening many tabs.
- Use a collision-proof dedup key.
- Add noopener/noreferrer for safety (also null the opener).
handleOpenAllPointers(columnName) { const data = this.tableData(); const pointers = data .map(row => row[columnName]) .filter(value => value && value.__type === 'Pointer' && value.className && value.objectId); // Open each unique pointer in a new tab const uniquePointers = new Map(); pointers.forEach(pointer => { - const key = `${pointer.className}-${pointer.objectId}`; + const key = JSON.stringify([pointer.className, pointer.objectId]); if (!uniquePointers.has(key)) { uniquePointers.set(key, pointer); } }); if (uniquePointers.size === 0) { this.showNote('No pointers found in this column', true); return; } const pointersArray = Array.from(uniquePointers.values()); + const MAX_TABS_WITHOUT_CONFIRM = 10; + if (pointersArray.length > MAX_TABS_WITHOUT_CONFIRM) { + const ok = window.confirm(`Open ${pointersArray.length} tabs?`); + if (!ok) return; + } // Open all tabs immediately to maintain user activation context let successCount = 0; pointersArray.forEach((pointer) => { try { const filters = JSON.stringify([{ field: 'objectId', constraint: 'eq', compareTo: pointer.objectId }]); const url = generatePath( this.context, `browser/${pointer.className}?filters=${encodeURIComponent(filters)}`, true ); - const newWindow = window.open(url, '_blank'); + const newWindow = window.open(url, '_blank', 'noopener,noreferrer'); if (newWindow) { + try { newWindow.opener = null; } catch(e) {} // best-effort successCount++; } } catch (error) { console.error('Failed to open tab for pointer:', pointer, error); } });If you prefer, I can also add an alternative action to open a single Browser tab with a multi-ID filter instead of many tabs.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/dashboard/Data/Views/Views.react.js
(3 hunks)src/dashboard/Data/Views/Views.scss
(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
src/dashboard/Data/Views/Views.react.js (2)
src/components/Icon/Icon.react.js (1)
Icon
(12-29)src/lib/generatePath.js (2)
filters
(8-8)generatePath
(3-34)
🪛 GitHub Check: Lint
src/dashboard/Data/Views/Views.react.js
[failure] 559-559:
Trailing spaces not allowed
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (1)
src/dashboard/Data/Views/Views.react.js (1)
280-283
: LGTM: upgrading to Pointer when detectedGood call to promote the column type when a pointer value appears.
.headerText { | ||
display: flex; | ||
align-items: center; | ||
justify-content: space-between; | ||
width: 100%; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Ensure header text truncates correctly with the new flex wrapper
Wrapping the header in a flex container can break the existing ellipsis. Add a dedicated, shrinkable label element and min-width: 0 to the flex container.
.headerText {
display: flex;
align-items: center;
justify-content: space-between;
width: 100%;
+ min-width: 0; /* allow children to shrink */
}
+
+.headerLabel {
+ overflow: hidden;
+ text-overflow: ellipsis;
+ white-space: nowrap;
+ min-width: 0;
+}
📝 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.
.headerText { | |
display: flex; | |
align-items: center; | |
justify-content: space-between; | |
width: 100%; | |
} | |
.headerText { | |
display: flex; | |
align-items: center; | |
justify-content: space-between; | |
width: 100%; | |
min-width: 0; /* allow children to shrink */ | |
} | |
.headerLabel { | |
overflow: hidden; | |
text-overflow: ellipsis; | |
white-space: nowrap; | |
min-width: 0; | |
} |
🤖 Prompt for AI Agents
In src/dashboard/Data/Views/Views.scss around lines 21–26, the new .headerText
flex wrapper prevents the header's text from truncating with an ellipsis; add
min-width: 0 to the .headerText container and introduce a dedicated, shrinkable
label element inside the header (make the label take remaining space and allow
shrinking, then set overflow: hidden, text-overflow: ellipsis and white-space:
nowrap on that label) and update the header markup to put the title text in that
label so the ellipsis works correctly.
.pointerIcon { | ||
cursor: pointer; | ||
opacity: 0.7; | ||
transition: opacity 0.2s ease; | ||
z-index: 10; | ||
pointer-events: auto; | ||
display: inline-flex; | ||
align-items: center; | ||
justify-content: center; | ||
height: 20px; | ||
width: 20px; | ||
background: rgba(255, 255, 255, 0.2); | ||
border-radius: 50%; | ||
margin-left: 5px; | ||
flex-shrink: 0; | ||
|
||
& svg { | ||
transform: rotate(316deg); | ||
} | ||
|
||
&:hover { | ||
opacity: 1; | ||
background: rgba(255, 255, 255, 0.3); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Make the header icon keyboard-accessible and clearly focusable
The interactive icon is a span with click only; use a button and add focus styles for accessibility.
.pointerIcon {
cursor: pointer;
opacity: 0.7;
transition: opacity 0.2s ease;
z-index: 10;
pointer-events: auto;
- display: inline-flex;
+ display: inline-flex;
align-items: center;
justify-content: center;
height: 20px;
width: 20px;
background: rgba(255, 255, 255, 0.2);
border-radius: 50%;
margin-left: 5px;
flex-shrink: 0;
+ border: 0; /* button reset */
+ outline: none; /* rely on :focus-visible below */
+ appearance: none;
& svg {
transform: rotate(316deg);
}
&:hover {
opacity: 1;
background: rgba(255, 255, 255, 0.3);
}
+
+ &:focus-visible {
+ outline: 2px solid #fff;
+ outline-offset: 2px;
+ opacity: 1;
+ }
}
📝 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.
.pointerIcon { | |
cursor: pointer; | |
opacity: 0.7; | |
transition: opacity 0.2s ease; | |
z-index: 10; | |
pointer-events: auto; | |
display: inline-flex; | |
align-items: center; | |
justify-content: center; | |
height: 20px; | |
width: 20px; | |
background: rgba(255, 255, 255, 0.2); | |
border-radius: 50%; | |
margin-left: 5px; | |
flex-shrink: 0; | |
& svg { | |
transform: rotate(316deg); | |
} | |
&:hover { | |
opacity: 1; | |
background: rgba(255, 255, 255, 0.3); | |
} | |
} | |
.pointerIcon { | |
cursor: pointer; | |
opacity: 0.7; | |
transition: opacity 0.2s ease; | |
z-index: 10; | |
pointer-events: auto; | |
display: inline-flex; | |
align-items: center; | |
justify-content: center; | |
height: 20px; | |
width: 20px; | |
background: rgba(255, 255, 255, 0.2); | |
border-radius: 50%; | |
margin-left: 5px; | |
flex-shrink: 0; | |
border: 0; /* button reset */ | |
outline: none; /* rely on :focus-visible below */ | |
appearance: none; | |
& svg { | |
transform: rotate(316deg); | |
} | |
&:hover { | |
opacity: 1; | |
background: rgba(255, 255, 255, 0.3); | |
} | |
&:focus-visible { | |
outline: 2px solid #fff; | |
outline-offset: 2px; | |
opacity: 1; | |
} | |
} |
🤖 Prompt for AI Agents
In src/dashboard/Data/Views/Views.scss around lines 28 to 52, the interactive
header icon is styled as a non-semantic span and lacks keyboard accessibility
and visible focus; change the markup to a button (or add role/button and
keyboard handlers if markup change isn't possible) and update the CSS selectors
to target button (or [role="button"]) instead of .pointerIcon, include
focus-visible/focus styles (outline or box-shadow and increased
background/opacity) and ensure pointer-events and display remain correct so the
element is operable via keyboard and shows a clear focus indicator.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/dashboard/Data/Views/Views.react.js (3)
280-283
: Upgrade-to-Pointer: set a sane min column width to avoid Pill truncation.
When a column upgrades to Pointer, enforce a minimum width so the Pill isn’t overly clipped.- } else if (type === 'Pointer' && columns[key].type !== 'Pointer') { - // If we find a pointer value, upgrade the column type to Pointer - columns[key].type = 'Pointer'; + } else if (type === 'Pointer' && columns[key].type !== 'Pointer') { + // If we find a pointer value, upgrade the column type to Pointer + columns[key].type = 'Pointer'; + columns[key].width = Math.max(columns[key].width, 120);
556-590
: Good a11y on the header button; disable during loading to prevent repeated mass-opens.
Prevents accidental double-clicks while data is still loading.- <button + <button type="button" className={styles.pointerIcon} + disabled={this.state.loading || !Array.isArray(this.state.data) || this.state.data.length === 0} onClick={(e) => { e.stopPropagation(); e.preventDefault(); this.handleOpenAllPointers(name); // Remove focus after action to follow UX best practices e.currentTarget.blur(); }} aria-label={`Open all pointers in ${name} column in new tabs`} - title="Open all pointers in new tabs" + title={this.state.loading ? 'Loading…' : 'Open all pointers in new tabs'} >
867-926
: Optional: handle arrays-of-pointers in a cell.
If a view column contains an array of Pointers, include them too.- const pointers = data - .map(row => row[columnName]) - .filter(value => value && value.__type === 'Pointer' && value.className && value.objectId); + const cellValues = data.map(row => row[columnName]); + const pointers = cellValues + .reduce((acc, v) => acc.concat(Array.isArray(v) ? v : [v]), []) + .filter(value => value && value.__type === 'Pointer' && value.className && value.objectId);
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
src/dashboard/Data/Views/Views.react.js
(3 hunks)src/dashboard/Data/Views/Views.scss
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- src/dashboard/Data/Views/Views.scss
🧰 Additional context used
🧬 Code graph analysis (1)
src/dashboard/Data/Views/Views.react.js (2)
src/components/Icon/Icon.react.js (1)
Icon
(12-29)src/lib/generatePath.js (2)
filters
(8-8)generatePath
(3-34)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Docker linux/amd64
🔇 Additional comments (1)
src/dashboard/Data/Views/Views.react.js (1)
858-860
: Nice security hardening with noopener/noreferrer.
Prevents opener hijacking and referrer leakage for Cmd-click behavior.
handleOpenAllPointers(columnName) { | ||
const data = this.tableData(); | ||
const pointers = data | ||
.map(row => row[columnName]) | ||
.filter(value => value && value.__type === 'Pointer' && value.className && value.objectId); | ||
|
||
// Open each unique pointer in a new tab | ||
const uniquePointers = new Map(); | ||
pointers.forEach(pointer => { | ||
// Use a more collision-proof key format with explicit separators | ||
const key = `className:${pointer.className}|objectId:${pointer.objectId}`; | ||
if (!uniquePointers.has(key)) { | ||
uniquePointers.set(key, pointer); | ||
} | ||
}); | ||
|
||
if (uniquePointers.size === 0) { | ||
this.showNote('No pointers found in this column', true); | ||
return; | ||
} | ||
|
||
const pointersArray = Array.from(uniquePointers.values()); | ||
|
||
// Confirm for large numbers of tabs to prevent overwhelming the user | ||
if (pointersArray.length > 10) { | ||
const confirmMessage = `This will open ${pointersArray.length} new tabs. This might overwhelm your browser. Continue?`; | ||
if (!confirm(confirmMessage)) { | ||
return; | ||
} | ||
} | ||
|
||
// Open all tabs immediately to maintain user activation context | ||
let errorCount = 0; | ||
|
||
pointersArray.forEach((pointer) => { | ||
try { | ||
const filters = JSON.stringify([{ field: 'objectId', constraint: 'eq', compareTo: pointer.objectId }]); | ||
const url = generatePath( | ||
this.context, | ||
`browser/${pointer.className}?filters=${encodeURIComponent(filters)}`, | ||
true | ||
); | ||
window.open(url, '_blank', 'noopener,noreferrer'); | ||
// Note: window.open with security attributes may return null even when successful, | ||
// so we assume success unless an exception is thrown | ||
} catch (error) { | ||
console.error('Failed to open tab for pointer:', pointer, error); | ||
errorCount++; | ||
} | ||
}); | ||
|
||
// Show result notification | ||
if (errorCount === 0) { | ||
this.showNote(`Opened ${pointersArray.length} pointer${pointersArray.length > 1 ? 's' : ''} in new tab${pointersArray.length > 1 ? 's' : ''}`, false); | ||
} else if (errorCount < pointersArray.length) { | ||
this.showNote(`Opened ${pointersArray.length - errorCount} of ${pointersArray.length} tabs. ${errorCount} failed to open.`, true); | ||
} else { | ||
this.showNote('Unable to open tabs. Please allow popups for this site and try again.', true); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🛠️ Refactor suggestion
Treat popup-blocker null returns as failures; guard on empty/non-array data.
Today you only count exceptions; most blockers return null
without throwing, leading to false “Opened N…” notes.
handleOpenAllPointers(columnName) {
- const data = this.tableData();
+ const data = this.tableData();
+ if (!Array.isArray(data) || data.length === 0) {
+ this.showNote('No data loaded', true);
+ return;
+ }
@@
- pointersArray.forEach((pointer) => {
+ pointersArray.forEach((pointer) => {
try {
const filters = JSON.stringify([{ field: 'objectId', constraint: 'eq', compareTo: pointer.objectId }]);
const url = generatePath(
this.context,
`browser/${pointer.className}?filters=${encodeURIComponent(filters)}`,
true
);
- window.open(url, '_blank', 'noopener,noreferrer');
- // Note: window.open with security attributes may return null even when successful,
- // so we assume success unless an exception is thrown
+ const win = window.open(url, '_blank', 'noopener,noreferrer');
+ if (!win) {
+ // Likely blocked by a popup blocker
+ errorCount++;
+ }
} catch (error) {
console.error('Failed to open tab for pointer:', pointer, error);
errorCount++;
}
});
📝 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.
handleOpenAllPointers(columnName) { | |
const data = this.tableData(); | |
const pointers = data | |
.map(row => row[columnName]) | |
.filter(value => value && value.__type === 'Pointer' && value.className && value.objectId); | |
// Open each unique pointer in a new tab | |
const uniquePointers = new Map(); | |
pointers.forEach(pointer => { | |
// Use a more collision-proof key format with explicit separators | |
const key = `className:${pointer.className}|objectId:${pointer.objectId}`; | |
if (!uniquePointers.has(key)) { | |
uniquePointers.set(key, pointer); | |
} | |
}); | |
if (uniquePointers.size === 0) { | |
this.showNote('No pointers found in this column', true); | |
return; | |
} | |
const pointersArray = Array.from(uniquePointers.values()); | |
// Confirm for large numbers of tabs to prevent overwhelming the user | |
if (pointersArray.length > 10) { | |
const confirmMessage = `This will open ${pointersArray.length} new tabs. This might overwhelm your browser. Continue?`; | |
if (!confirm(confirmMessage)) { | |
return; | |
} | |
} | |
// Open all tabs immediately to maintain user activation context | |
let errorCount = 0; | |
pointersArray.forEach((pointer) => { | |
try { | |
const filters = JSON.stringify([{ field: 'objectId', constraint: 'eq', compareTo: pointer.objectId }]); | |
const url = generatePath( | |
this.context, | |
`browser/${pointer.className}?filters=${encodeURIComponent(filters)}`, | |
true | |
); | |
window.open(url, '_blank', 'noopener,noreferrer'); | |
// Note: window.open with security attributes may return null even when successful, | |
// so we assume success unless an exception is thrown | |
} catch (error) { | |
console.error('Failed to open tab for pointer:', pointer, error); | |
errorCount++; | |
} | |
}); | |
// Show result notification | |
if (errorCount === 0) { | |
this.showNote(`Opened ${pointersArray.length} pointer${pointersArray.length > 1 ? 's' : ''} in new tab${pointersArray.length > 1 ? 's' : ''}`, false); | |
} else if (errorCount < pointersArray.length) { | |
this.showNote(`Opened ${pointersArray.length - errorCount} of ${pointersArray.length} tabs. ${errorCount} failed to open.`, true); | |
} else { | |
this.showNote('Unable to open tabs. Please allow popups for this site and try again.', true); | |
} | |
} | |
handleOpenAllPointers(columnName) { | |
const data = this.tableData(); | |
if (!Array.isArray(data) || data.length === 0) { | |
this.showNote('No data loaded', true); | |
return; | |
} | |
const pointers = data | |
.map(row => row[columnName]) | |
.filter(value => value && value.__type === 'Pointer' && value.className && value.objectId); | |
// Open each unique pointer in a new tab | |
const uniquePointers = new Map(); | |
pointers.forEach(pointer => { | |
// Use a more collision-proof key format with explicit separators | |
const key = `className:${pointer.className}|objectId:${pointer.objectId}`; | |
if (!uniquePointers.has(key)) { | |
uniquePointers.set(key, pointer); | |
} | |
}); | |
if (uniquePointers.size === 0) { | |
this.showNote('No pointers found in this column', true); | |
return; | |
} | |
const pointersArray = Array.from(uniquePointers.values()); | |
// Confirm for large numbers of tabs to prevent overwhelming the user | |
if (pointersArray.length > 10) { | |
const confirmMessage = `This will open ${pointersArray.length} new tabs. This might overwhelm your browser. Continue?`; | |
if (!confirm(confirmMessage)) { | |
return; | |
} | |
} | |
// Open all tabs immediately to maintain user activation context | |
let errorCount = 0; | |
pointersArray.forEach((pointer) => { | |
try { | |
const filters = JSON.stringify([{ field: 'objectId', constraint: 'eq', compareTo: pointer.objectId }]); | |
const url = generatePath( | |
this.context, | |
`browser/${pointer.className}?filters=${encodeURIComponent(filters)}`, | |
true | |
); | |
- window.open(url, '_blank', 'noopener,noreferrer'); | |
- // Note: window.open with security attributes may return null even when successful, | |
const win = window.open(url, '_blank', 'noopener,noreferrer'); | |
if (!win) { | |
// Likely blocked by a popup blocker | |
errorCount++; | |
} | |
} catch (error) { | |
console.error('Failed to open tab for pointer:', pointer, error); | |
errorCount++; | |
} | |
}); | |
// Show result notification | |
if (errorCount === 0) { | |
this.showNote(`Opened ${pointersArray.length} pointer${pointersArray.length > 1 ? 's' : ''} in new tab${pointersArray.length > 1 ? 's' : ''}`, false); | |
} else if (errorCount < pointersArray.length) { | |
this.showNote(`Opened ${pointersArray.length - errorCount} of ${pointersArray.length} tabs. ${errorCount} failed to open.`, true); | |
} else { | |
this.showNote('Unable to open tabs. Please allow popups for this site and try again.', true); | |
} | |
} |
# [7.5.0-alpha.1](7.4.0...7.5.0-alpha.1) (2025-09-09) ### Features * Add button to view table to open all pointers of a column in new browser tabs ([#2976](#2976)) ([b8033a4](b8033a4))
🎉 This change has been released in version 7.5.0-alpha.1 |
New Pull Request Checklist
Issue Description
If a user wants to open each pointer in a view table column in a new browser tab, the user needs to click on each pointer individually.
Approach
Add icon to batch-open pointers in a view table column in new browser tabs.
Summary by CodeRabbit
New Features
Style