Island rhythms/array facelift #119

IslandRhythms · 2025-12-02T22:38:20Z

Copilot

Pull request overview

This PR refactors the array display and editing functionality across the frontend, implementing a new visual design for array components. The changes move away from eval()-based parsing to JSON.parse() for security and replace code-highlighted displays with styled list views featuring individual item cards.

Key Changes:

Replaced eval() with JSON.parse() in edit-array component for safer array parsing
Implemented new card-based UI for displaying array items with index labels and hover effects
Added truncation support for arrays in document-property with "Show all" expansion

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated 13 comments.

Show a summary per file

File	Description
frontend/src/edit-array/edit-array.js	Refactored to use JSON.parse instead of eval, reorganized lifecycle methods and watchers
frontend/src/edit-array/edit-array.html	Simplified template to single textarea with updated styling attributes
frontend/src/edit-array/edit-array.css	Added extensive styling for array items (though most appear unused with current template)
frontend/src/document-details/document-property/document-property.js	Added array detection, truncation logic, and formatting methods for array display
frontend/src/document-details/document-property/document-property.html	Added specialized truncated array view with expandable "Show all" functionality
frontend/src/document-details/document-property/document-property.css	Added styling for truncated array view matching detail-array design
frontend/src/detail-array/detail-array.js	Replaced Prism syntax highlighting with structured array rendering and formatting methods
frontend/src/detail-array/detail-array.html	Replaced code block with card-based array item display
frontend/src/detail-array/detail-array.css	Added comprehensive styling for array item cards with index labels and hover effects

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-12-02T22:43:16Z

frontend/src/edit-array/edit-array.css

+.array-items {
+  margin-top: 8px;
+}
+
+.array-item {
+  margin-bottom: 6px;
+  padding: 10px 12px 10px 16px;
+  background: transparent;
+  border: none;
+  border-left: 3px solid #3b82f6;
+  border-radius: 0;
+  transition: all 0.2s;
+  cursor: pointer;
+}
+
+.array-item:last-child {
+  margin-bottom: 0;
+}
+
+.array-item:hover {
+  background: #f8fafc;
+  border-left-color: #2563eb;
+}
+
+.array-item-header {
+  display: flex;
+  align-items: center;
+  justify-content: flex-end;
+  margin-bottom: 8px;
+  padding-bottom: 0;
+  border-bottom: none;
+}
+
+.array-item-index {
+  font-size: 11px;
+  color: #6b7280;
+  font-family: monospace;
+  font-weight: 600;
+  background: #f3f4f6;
+  padding: 2px 6px;
+  border-radius: 3px;
+  white-space: nowrap;
+  flex-shrink: 0;
+}
+
+.array-item-header-actions {
+  display: flex;
+  align-items: center;
+  gap: 6px;
+}
+
+.array-item-action-button {
+  color: #6b7280;
+  opacity: 0.7;
+  padding: 5px 7px;
+  border-radius: 4px;
+  transition: all 0.15s;
+  display: inline-flex;
+  align-items: center;
+  justify-content: center;
+  font-size: 12px;
+  background: transparent;
+  border: none;
+  cursor: pointer;
+  flex-shrink: 0;
+}
+
+.array-item-action-button:hover {
+  opacity: 1;
+  background-color: #f3f4f6;
+}
+
+.array-item-remove {
+  color: #dc2626;
+}
+
+.array-item-remove:hover {
+  background-color: #fee2e2;
+}
+
+
+.array-item-form-fields {
+  display: flex;
+  flex-direction: column;
+  gap: 8px;
+  padding: 0;
+  background: transparent;
+  border: none;
+  margin-top: 4px;
+}
+
+.array-form-field {
+  display: flex;
+  align-items: center;
+  gap: 8px;
+}
+
+.array-form-label {
+  font-size: 11px;
+  color: #4b5563;
+  font-weight: 600;
+  min-width: 80px;
+  text-align: left;
+  font-family: monospace;
+  padding: 2px 0;
+  flex-shrink: 0;
+}
+
+.array-form-input {
+  flex: 1;
+  font-size: 12px;
+  padding: 5px 8px;
+  border: 1px solid #d1d5db;
+  border-radius: 3px;
+  font-family: monospace;
+  background: white;
+  transition: all 0.15s;
+}
+
+.array-form-input:focus {
+  outline: none;
+  border-color: #3b82f6;
+  box-shadow: 0 0 0 2px rgba(59, 130, 246, 0.1);
+}
+
+.array-item-simple {
+  display: flex;
+  flex-direction: column;
+  gap: 8px;
+  margin-top: 4px;
+}
+
+.array-simple-input {
+  flex: 1;
+  font-size: 12px;
+  padding: 5px 8px;
+  border: 1px solid #d1d5db;
+  border-radius: 3px;
+  font-family: monospace;
+  background: white;
+  transition: all 0.15s;
+}
+
+.array-simple-input:focus {
+  outline: none;
+  border-color: #3b82f6;
+  box-shadow: 0 0 0 2px rgba(59, 130, 246, 0.1);
+}
+
+.array-add-button {
+  display: inline-flex;
+  align-items: center;
+  gap: 6px;
+  padding: 8px 16px;
+  font-size: 13px;
+  font-weight: 600;
+  color: white;
+  background: #3b82f6;
+  border: none;
+  border-radius: 6px;
+  cursor: pointer;
+  transition: all 0.2s;
+  margin-bottom: 12px;
+  box-shadow: 0 1px 2px rgba(0, 0, 0, 0.05);
+}
+
+.array-add-button:hover {
+  background: #2563eb;
+  box-shadow: 0 2px 4px rgba(0, 0, 0, 0.1);
+  transform: translateY(-1px);
+}
+
+.array-add-button:active {
+  transform: translateY(0);
+  box-shadow: 0 1px 2px rgba(0, 0, 0, 0.05);
+}
+
+.array-add-button svg {
+  width: 16px;
+  height: 16px;
+}
+
+.add-item-form {
+  display: flex;
+  flex-direction: column;
+  gap: 12px;
+}
+
+.add-item-form .array-form-field {
+  display: flex;
+  align-items: center;
+  gap: 12px;
+}
+
+.add-item-form .array-form-label {
+  min-width: 100px;
+  font-size: 12px;
+}
+
+.add-item-form .array-form-input {
+  font-size: 13px;
+  padding: 6px 10px;
 }


The CSS file contains extensive styling for array items (.array-items, .array-item, .array-form-field, .array-add-button, etc.) but the HTML template only renders a single CodeMirror textarea. These CSS classes appear to be unused, which suggests either incomplete implementation or leftover code from a previous approach. Consider removing unused styles or ensuring the HTML template uses these classes.

Copilot · 2025-12-02T22:43:16Z

frontend/src/edit-array/edit-array.js

 const appendCSS = require('../appendCSS');
 appendCSS(require('./edit-array.css'));

 module.exports = app => app.component('edit-array', {
  template: template,
  props: ['value'],
-  data: () => ({ currentValue: -1 }),
-  mounted() {
-    this.currentValue = this.value == null
-      ? '' + this.value
-      : JSON.stringify(this.value, null, '  ').trim();
-    this.$refs.arrayEditor.value = this.currentValue;
-    this.editor = CodeMirror.fromTextArea(this.$refs.arrayEditor, {
-      mode: 'javascript',
-      lineNumbers: true
-    });
-    this.editor.on('change', ev => {
-      this.currentValue = this.editor.getValue();
-    });
+  data() {


The removed ObjectId proxy was likely used to support ObjectId creation in the eval-based array editing. Since the code now uses JSON.parse instead of eval, this import is correctly removed. However, this means the new implementation may not support creating MongoDB ObjectIds in the array editor. Consider whether ObjectId support is needed, and if so, add custom parsing logic to handle ObjectId strings.

Copilot · 2025-12-02T22:43:16Z

frontend/src/edit-array/edit-array.js

+      // Update CodeMirror editor if it exists
+      this.$nextTick(() => {
+        if (this.arrayEditor) {
+          const arrayStr = JSON.stringify(this.arrayValue, null, 2);
+          this.arrayEditor.setValue(arrayStr);
+        }
+      });


The initializeArray method updates the CodeMirror editor in a watcher callback, which can lead to redundant updates. This method is called from both mounted() and the value watcher. When the component first mounts with an initial value, the watcher with immediate: true will trigger before the editor is initialized, then mounted() will initialize the editor and call initializeArray() again. Consider restructuring to avoid this redundancy.

Copilot · 2025-12-02T22:43:17Z

frontend/src/detail-array/detail-array.js

+    formatValue(item) {
+      if (item == null) {
+        return 'null';
+      }
+      if (typeof item === 'object') {
+        return inspect(item, { maxArrayLength: 50 });
+      }
+      return String(item);
+    },
+    isObjectItem(item) {
+      return item != null && typeof item === 'object' && !Array.isArray(item) && item.constructor === Object;
+    },
+    getItemKeys(item) {
+      if (!this.isObjectItem(item)) {
+        return [];
+      }
+      return Object.keys(item);
+    },
+    formatItemValue(item, key) {
+      const value = item[key];
+      if (value === null || value === undefined) {
+        return 'null';
+      }
+      if (typeof value === 'object') {
+        return inspect(value, { maxArrayLength: 50 });
+      }
+      return String(value);
+    },


The methods formatValue, isObjectItem, getItemKeys, and formatItemValue are duplicated across multiple components (detail-array.js and document-property.js have identical implementations). Consider extracting these utility functions into a shared module to reduce code duplication and improve maintainability.

Copilot · 2025-12-02T22:43:17Z

frontend/src/detail-array/detail-array.js

+  watch: {
+    value: {
+      handler(newValue) {
+        this.initializeArray();
+      },
+      deep: true,
+      immediate: true
+    }


The watcher has immediate: true which will trigger before mounted() is called. This means initializeArray() will be called twice on component initialization: once from the immediate watcher and once from mounted(). The second call in mounted() is redundant. Consider removing the call from mounted() or removing immediate: true from the watcher.

Copilot · 2025-12-02T22:43:17Z

frontend/src/document-details/document-property/document-property.js

+    formatArrayItem(item) {
+      if (item == null) {
+        return 'null';
+      }
+      if (typeof item === 'object') {
+        return inspect(item, { maxArrayLength: 50 });
+      }
+      return String(item);
+    },
+    isObjectItem(item) {
+      return item != null && typeof item === 'object' && !Array.isArray(item) && item.constructor === Object;
+    },
+    getItemKeys(item) {
+      if (!this.isObjectItem(item)) {
+        return [];
+      }
+      return Object.keys(item);
+    },
+    formatItemValue(item, key) {
+      const value = item[key];
+      if (value === null || value === undefined) {
+        return 'null';
+      }
+      if (typeof value === 'object') {
+        return inspect(value, { maxArrayLength: 50 });
+      }
+      return String(value);
+    },


The methods formatArrayItem, isObjectItem, getItemKeys, and formatItemValue are duplicated from detail-array.js. Consider extracting these utility functions into a shared module to reduce code duplication and improve maintainability.

Copilot · 2025-12-02T22:43:18Z

frontend/src/document-details/document-property/document-property.js

+      // For arrays, show truncated view if needs truncation and not expanded
+      if (this.isArray) {
+        return this.needsTruncation && !this.isValueExpanded;
+      }
+      // For other types, show truncated if needs truncation and not expanded


The shouldShowTruncated computed property has redundant logic. Both branches (lines 63-65 and 67) return the same expression. This can be simplified to just return this.needsTruncation && !this.isValueExpanded;

Suggested change

// For arrays, show truncated view if needs truncation and not expanded

if (this.isArray) {

return this.needsTruncation && !this.isValueExpanded;

}

// For other types, show truncated if needs truncation and not expanded

// Show truncated view if needs truncation and not expanded

Copilot · 2025-12-02T22:43:18Z

frontend/src/document-details/document-property/document-property.js

+    isArray() {
+      const value = this.getValueForPath(this.path.path);
+      return Array.isArray(value);
+    },


The isArray computed property would be more efficient if it cached the path value. Currently, it calls this.getValueForPath(this.path.path) every time, and this same call is repeated in arrayValue (line 48). Consider caching the value or combining these computed properties to avoid redundant lookups.

Copilot · 2025-12-02T22:43:18Z

frontend/src/detail-array/detail-array.css

+.array-item {
+  margin-bottom: 6px;
+  padding: 10px 12px 10px 16px;
+  background: transparent;
+  border: none;
+  border-left: 3px solid #3b82f6;
+  border-radius: 0;
+  transition: all 0.2s;
+  cursor: pointer;
+  position: relative;
+}
+
+.array-item-index-label {
+  position: absolute;
+  left: -8px;
+  top: 10px;
+  width: 20px;
+  height: 20px;
+  background: #3b82f6;
+  color: white;
+  border-radius: 50%;
+  display: flex;
+  align-items: center;
+  justify-content: center;
+  font-size: 10px;
+  font-weight: 600;
+  font-family: monospace;
+  z-index: 1;
+}
+
+.array-item:last-child {
+  margin-bottom: 0;
+}
+
+.array-item:hover {
+  background: #f8fafc;
+  border-left-color: #2563eb;
+  box-shadow: 0 4px 12px rgba(0, 0, 0, 0.15);
+  transform: translateY(-2px);
+}
+
+.array-item:hover .array-item-index-label {
+  background: #2563eb;
+}
+
+
+.array-item-value {
+  font-size: 12px;
+  padding: 5px 8px;
+  font-family: monospace;
+  color: #1f2937;
+  word-break: break-word;
+  white-space: pre-wrap;
+  margin-top: 4px;
+}
+
+.array-item-fields {
+  display: flex;
+  flex-direction: column;
+  gap: 4px;
+  margin-top: 4px;
+  padding: 0 8px;
+}
+
+.array-item-field {
+  display: flex;
+  align-items: flex-start;
+  gap: 8px;
+  font-size: 12px;
+  font-family: monospace;
+}
+
+.array-item-field-label {
+  font-weight: 600;
+  color: #4b5563;
+  flex-shrink: 0;
+  min-width: 80px;
+}
+
+.array-item-field-value {
+  color: #1f2937;
+  word-break: break-word;
+  white-space: pre-wrap;
+  flex: 1;
+}
+


There is significant CSS duplication between detail-array.css and document-property.css (array-truncated-view section). The styles for .array-item, .array-item-index-label, .array-item-value, .array-item-fields, .array-item-field, .array-item-field-label, and .array-item-field-value are nearly identical. Consider extracting these into a shared CSS file to maintain consistency and reduce duplication.

Suggested change

.array-item {

margin-bottom: 6px;

padding: 10px 12px 10px 16px;

background: transparent;

border: none;

border-left: 3px solid #3b82f6;

border-radius: 0;

transition: all 0.2s;

cursor: pointer;

position: relative;

}

.array-item-index-label {

position: absolute;

left: -8px;

top: 10px;

width: 20px;

height: 20px;

background: #3b82f6;

color: white;

border-radius: 50%;

display: flex;

align-items: center;

justify-content: center;

font-size: 10px;

font-weight: 600;

font-family: monospace;

z-index: 1;

}

.array-item:last-child {

margin-bottom: 0;

}

.array-item:hover {

background: #f8fafc;

border-left-color: #2563eb;

box-shadow: 0 4px 12px rgba(0, 0, 0, 0.15);

transform: translateY(-2px);

}

.array-item:hover .array-item-index-label {

background: #2563eb;

}

.array-item-value {

font-size: 12px;

padding: 5px 8px;

font-family: monospace;

color: #1f2937;

word-break: break-word;

white-space: pre-wrap;

margin-top: 4px;

}

.array-item-fields {

display: flex;

flex-direction: column;

gap: 4px;

margin-top: 4px;

padding: 0 8px;

}

.array-item-field {

display: flex;

align-items: flex-start;

gap: 8px;

font-size: 12px;

font-family: monospace;

}

.array-item-field-label {

font-weight: 600;

color: #4b5563;

flex-shrink: 0;

min-width: 80px;

}

.array-item-field-value {

color: #1f2937;

word-break: break-word;

white-space: pre-wrap;

flex: 1;

}

/* The styles for .array-item, .array-item-index-label, .array-item-value, .array-item-fields,

.array-item-field, .array-item-field-label, and .array-item-field-value have been moved to array-shared.css

to reduce duplication and maintain consistency across components. */

Copilot · 2025-12-02T22:43:18Z

frontend/src/document-details/document-property/document-property.html

+        <button
+          @click="toggleValueExpansion"
+          class="mt-2 text-blue-600 hover:text-blue-800 text-sm font-medium flex items-center gap-1 transform transition-all duration-200 ease-in-out hover:translate-x-0.5"
+        >
+          <svg class="w-4 h-4" fill="none" stroke="currentColor" viewBox="0 0 24 24">
+            <path stroke-linecap="round" stroke-linejoin="round" stroke-width="2" d="M19 9l-7 7-7-7"></path>
+          </svg>
+          Show all {{ arrayValue.length }} items
+        </button>


The "Show all items" button lacks an accessible label that describes what will happen. Consider adding an aria-label attribute that provides more context, such as aria-label="Expand to show all array items".

IslandRhythms · 2025-12-02T22:49:27Z

Given what arrays are there isn't as much as I thought I could do with them. For example, What if an array of objects contains an array of objects etc. I wanted to try adding a remove button but it would conflict with how we have an editting state. I can dedicate more time to this if you wish but for now this is what I've got.

IslandRhythms added 5 commits December 1, 2025 18:22

begin array facelift

ce2ca38

good progress

8a22591

need todo more work on objectIds

0973a53

finishing touches

0050ce5

K.I.S.S.

bd4de94

IslandRhythms requested review from Copilot and vkarpov15 December 2, 2025 22:38

Copilot started reviewing on behalf of IslandRhythms December 2, 2025 22:38 View session

Copilot finished reviewing on behalf of IslandRhythms December 2, 2025 22:40

Copilot AI reviewed Dec 2, 2025

View reviewed changes

Island rhythms/array facelift #119

Are you sure you want to change the base?

Island rhythms/array facelift #119

Uh oh!

Conversation

IslandRhythms commented Dec 2, 2025

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Dec 2, 2025

Choose a reason for hiding this comment

Uh oh!

IslandRhythms commented Dec 2, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants