Claude/property mgmt review uyt2t4 - #44
Merged
Merged
Conversation
Drove the app as a true first-time user would and found the guided Launch Plan dashboard (which is good) was followed immediately by a 24,846px wall of 262 tasks across 17 simultaneously-expanded departments, all glowing red UNOWNED. For someone unfamiliar with PM software or web apps generally, that reads as "this is broken," not "here's a checklist." - Departments now start collapsed instead of forced open on every load (collapsed headers already show task count and done/blocked totals, so nothing informational is lost). This also fixes an existing inconsistency: every other full re-render (auto-assign, demo load, undo/import) already collapsed everything since only the one-time initial load forced departments open — now all paths behave the same way. - Added an "Expand All / Collapse All" toggle to the filter bar as the one-click way back to the old always-open view. - Added "Show me an example first" as a second button directly in the onboarding modal, next to "Start Mapping" — skips straight to the demo company with no confirmation dialog (nothing to lose on a fresh workspace), so an anxious first-timer never has to face a truly empty state before deciding if this tool is for them. Verified live: fresh onboarding now renders a 6,663px page (was 24,846px) with 0 departments expanded; Expand All/Collapse All toggles correctly both directions and individual department clicks still work normally; "Show me an example first" closes the modal, loads the demo company, and leaves departments collapsed. Also fixed a CSS cascade bug caught in the same pass — a more specific pre-existing `.setup-form .onboarding-btn` rule was clobbering the new secondary button's styling before I scoped the override to match.
A company migrating off a spreadsheet already has this data in Excel or Sheets — typing each record into the form one at a time was the remaining piece of friction after the onboarding fixes. "Import CSV" next to each Portfolio form now bulk-adds records instead. - New parseCSV()/buildCsvHeaderMap() pure functions in utils.js/cjs: a small RFC4180-style parser handling quoted fields, embedded commas/newlines, escaped "" quotes, and CRLF/LF, plus case-insensitive header-name matching so columns can be reordered or partial. 20 new unit tests covering the parser's edge cases directly. - exportPropertiesCSV/exportTenantsCSV/exportVendorsCSV updated to include the rent/lease/balance/document-link fields added earlier this session (they'd been missed when those fields were added), so export -> edit in a spreadsheet -> re-import is a real round trip. - Tenant import resolves a typed property name to an existing property's ID by case-insensitive match; an unmatched name is left unassigned and counted as a warning rather than blocking the row. Rows with no name are skipped, not errored. Document links go through the same isSafeUrl() check as manual entry — verified live that a javascript: URL in a CSV cell is silently dropped rather than ever reaching an <a href>. - A confirmation summary (record count, skipped rows, warnings) is shown before anything is saved; import always adds new records and never edits or removes existing ones. A backup snapshot is taken first, same as JSON import. Verified end-to-end with real CSV fixtures covering the normal case, an unmatched property reference, a blank-name row, and a malicious javascript: URL — all handled correctly with zero console errors. Full test suite (85 passing) and production build clean.
hypnoticdata777
commented
Jul 7, 2026
hypnoticdata777
left a comment
hypnoticdata777
left a comment
Owner
Author
There was a problem hiding this comment.
On js/app.js:170 (the collapsed-departments comment)
Yeah, this was bugging me too — first thing a new user saw was a wall of red. Glad we fixed the actual default instead of just adding a tip telling people to collapse it themselves.
On index.html:262 ("Show me an example first" button)
Good call putting this right on the onboarding screen instead of burying it in the dashboard. Anyone nervous about an empty screen gets an out immediately.
On js/views/portfolio.js:444 (isSafeUrl re-check on the doc link)
Nice that this gets checked again here and not just when the form is submitted — covers CSV/import paths too, not just manual entry.
On js/views/portfolio.js:410 (pluralLabel)
Small thing but appreciate that "property/properties" got its own case instead of just tacking on an "s" — would've bugged me every time I saw "1 propertys" in the confirm dialog.
hypnoticdata777
added a commit
that referenced
this pull request
Jul 17, 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
2 participants
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.
No description provided.