Add shared material auto-fill and type-to-search across all writer pages#16
Merged
Conversation
- SharedJS.h: add loadMaterialDb(), lookupMaterial(), trackAutoFill(), autoFillMaterialData() with TigerTag API fetch + hardcoded fallback - All writer pages: material field changed from dropdown to type-to-search (input + datalist). API expands options when reachable. - All writer pages: selecting a material auto-fills temps (±10°C nozzle, ±5°C bed) and density. User edits are tracked and preserved. - TigerTag: brand field changed from dropdown to type-to-search - Hidden ID fields maintain numeric IDs for tag formats that need them (OpenPrintTag material_type, TigerTag material_id + brand_id) Closes #12
|
Note
|
| Cohort / File(s) | Summary |
|---|---|
Shared Material Infrastructure src/SharedJS.h |
Added global material database cache (_materialDb, _materialFallback, _materialDbLoaded), loadMaterialDb() to fetch materials from API endpoint, lookupMaterial(name) for normalized material name resolution with exact/dashed/prefix-matching rules, trackAutoFill(fieldIds) to mark fields as user-edited via listeners, and autoFillMaterialData(materialName, fieldMap) to populate temperature/density fields from material lookup results. |
Writer UI Migration src/OpenPrintTagWriterHTML.h, src/OpenTag3DWriterHTML.h, src/TigerTagWriterHTML.h, src/UIDRegistrationHTML.h |
Replaced fixed material <select> dropdowns with searchable <input> fields paired with <datalist> elements; added client-side JavaScript to synchronize material selections with hidden ID fields via syncMaterialId(), trigger auto-fill on material input events, dynamically load and merge material database options, and mark auto-filled fields to prevent overwriting manual edits. |
Sequence Diagram
sequenceDiagram
participant User
participant Form as Form UI
participant SharedJS as SharedJS Functions
participant API as Material API
participant DOM as DOM / Datalist
User->>Form: Select or type material name
Form->>SharedJS: input event → autoFillMaterialData(materialName)
SharedJS->>SharedJS: lookupMaterial(name) - normalize & search
alt Material found in cache
SharedJS->>Form: Return cached material data
else Cache empty or miss
SharedJS->>API: loadMaterialDb() fetch materials
API-->>SharedJS: Return material records
SharedJS->>DOM: Append new options to datalist
SharedJS->>SharedJS: lookupMaterial(name) retry
SharedJS->>Form: Return material data
end
Form->>Form: Check if field marked autoFilled
Form->>Form: Populate min_print_temp, max_print_temp, etc.
Form->>SharedJS: trackAutoFill(fieldIds) mark fields
SharedJS->>DOM: Register input/change listeners
Estimated code review effort
🎯 4 (Complex) | ⏱️ ~45 minutes
Possibly related PRs
- Fix TigerTag dropdown: fetch from TigerTag API with hardcoded fallback #10 — Modifies TigerTagWriterHTML.h to replace hardcoded material/brand options with dynamic API-loaded datalist entries and auto-fill behavior.
- Add NFC+ UID registration page for plain NFC tags #15 — Modifies UIDRegistrationHTML.h to implement material selection as searchable input with datalist and auto-fill synchronization logic.
🚥 Pre-merge checks | ✅ 4 | ❌ 1
❌ Failed checks (1 warning)
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage | Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
✅ Passed checks (4 passed)
| Check name | Status | Explanation |
|---|---|---|
| Title check | ✅ Passed | The title clearly and specifically describes the main change: converting material fields to type-to-search inputs with auto-fill functionality across all writer pages. |
| Description check | ✅ Passed | The description covers summary, changes, pages affected, and test plan; however, it's missing the 'How to Test' section format and omits compilation verification steps. |
| Linked Issues check | ✅ Passed | All objectives from issue #12 are met: material API reused across pages, nozzle/bed temps auto-filled (±10°C/±5°C), shared logic in SharedJS.h, and offline fallback provided. |
| Out of Scope Changes check | ✅ Passed | All changes directly support the linked issue #12 objectives: type-to-search conversion, auto-fill implementation, shared JS logic, and material database expansion. |
✏️ Tip: You can configure your own custom pre-merge checks in the settings.
✨ Finishing Touches
📝 Generate docstrings
- Create stacked PR
- Commit on current branch
🧪 Generate unit tests (beta)
- Create PR with unit tests
- Commit unit tests in branch
feature/auto-fill-temps
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 @coderabbitai help to get the list of available commands and usage tips.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/UIDRegistrationHTML.h (1)
341-341:⚠️ Potential issue | 🔴 CriticalRuntime error:
materialTypeElis now an input, not a select.After refactoring
material_typefrom a<select>to an<input>, this line accesses.optionsand.selectedIndexwhich don't exist on input elements. This will throw a TypeError whenregisterFlow()executes.🐛 Proposed fix
- var materialTypeText = materialTypeEl.options[materialTypeEl.selectedIndex].text; + var materialTypeText = materialTypeEl.value || 'PLA';🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/UIDRegistrationHTML.h` at line 341, The code in registerFlow() reads materialTypeEl.options[materialTypeEl.selectedIndex].text but materialTypeEl is now an <input>, causing a TypeError; change the logic that derives materialTypeText to use the input's value (e.g., materialTypeEl.value or materialTypeEl.value.trim()) instead of accessing .options/.selectedIndex, and update any downstream uses that expect a string accordingly so materialTypeText comes from materialTypeEl.value.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/SharedJS.h`:
- Around line 163-168: The then-callback that populates _materialDb assumes each
item has a string material and directly calls m.material.toUpperCase(), which
can throw if m.material is missing or not a string; update the loop in the
.then(function(data) { ... }) handler to guard each item (e.g., verify m is
truthy and typeof m.material === 'string' or use a safe access) before calling
toUpperCase(), and only add entries to _materialDb when the validated uppercase
key is non-empty to avoid null/undefined key errors.
---
Outside diff comments:
In `@src/UIDRegistrationHTML.h`:
- Line 341: The code in registerFlow() reads
materialTypeEl.options[materialTypeEl.selectedIndex].text but materialTypeEl is
now an <input>, causing a TypeError; change the logic that derives
materialTypeText to use the input's value (e.g., materialTypeEl.value or
materialTypeEl.value.trim()) instead of accessing .options/.selectedIndex, and
update any downstream uses that expect a string accordingly so materialTypeText
comes from materialTypeEl.value.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
Run ID: cd3768e3-44c0-4af1-95cf-d100c84f3f36
📒 Files selected for processing (5)
src/OpenPrintTagWriterHTML.hsrc/OpenTag3DWriterHTML.hsrc/SharedJS.hsrc/TigerTagWriterHTML.hsrc/UIDRegistrationHTML.h
Comment on lines
+163
to
+168
| .then(function(data) { | ||
| if (Array.isArray(data)) { | ||
| data.forEach(function(m) { | ||
| if (m.material) _materialDb[m.material.toUpperCase()] = m; | ||
| }); | ||
| } |
There was a problem hiding this comment.
Potential null reference if API returns malformed material data.
The code assumes m.material exists when iterating API results. If the API returns an object without a material property, m.material.toUpperCase() will throw a TypeError.
🛡️ Proposed defensive fix
if (Array.isArray(data)) {
data.forEach(function(m) {
- if (m.material) _materialDb[m.material.toUpperCase()] = m;
+ if (m && m.material) _materialDb[m.material.toUpperCase()] = m;
});
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/SharedJS.h` around lines 163 - 168, The then-callback that populates
_materialDb assumes each item has a string material and directly calls
m.material.toUpperCase(), which can throw if m.material is missing or not a
string; update the loop in the .then(function(data) { ... }) handler to guard
each item (e.g., verify m is truthy and typeof m.material === 'string' or use a
safe access) before calling toUpperCase(), and only add entries to _materialDb
when the validated uppercase key is non-empty to avoid null/undefined key
errors.
This was referenced Mar 29, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Pages affected
Test plan
Closes #12
Summary by CodeRabbit