Update documentation#165
Conversation
SyniRon
left a comment
There was a problem hiding this comment.
Good improvement overall thanks for taking the time. The old BilletBank instructions were genuinely out of date.
A few things to clean up before merge, mostly in the worked examples and a couple of claims that don't match what the code actually does. I've left them inline. I realize some of them are carryovers from what the readme already said but if we're updating might as well make sure its correct now.
Once those are sorted I think we're good to go.
| 1. **`client/app/adr/page.jsx`**: the ADR. Add the billet group's ID to a `units` entry's `selectors` (existing category) or add a new `units` entry (new category), as shown above. | ||
| 2. **`client/app/reusableModules/BilletBank.jsx`**: the data behind Roster Statistics. Add the billet ID to the matching array (existing category), or add new arrays + a group object for a new category, and export them at the bottom of the file. | ||
| 3. **`client/app/rosterstatistics/page.jsx`**: the Statistics layout. Add the unit to the relevant `<Statistics>` block's `billetIDs` and add a matching label to its `labelArray` (the two must stay the same length and order). | ||
| 4. **`client/app/rosterstatistics/modules/statistics.jsx`**: the chart colors. The regiment-wide chart colors its segments by position from a fixed `colors` array; if you added a segment to the regiment `billetIDs`, add a matching color here or the new segment renders with none. |
There was a problem hiding this comment.
This isn't quite what the chart does. ApexCharts doesn't leave the slice uncoloured, it loops back to the start of the colors array and reuses one, so the new segment comes out the same colour as an existing slice and they're hard to tell apart. The wording shows up a second time at L342 in the Statistics section, so worth fixing both so the reason matches what actually happens.
|
|
||
| ```javascript | ||
| const imoCommand = ["5", "9", "28"]; | ||
| { title: "Information Management Office Command", selectors: ["5", "9", "28"] }, |
There was a problem hiding this comment.
Couple of things with this example. The selectors in the real units array are numbers, not strings, so this'd be selectors: [5, 9, 28]. The "Add New Category" example just below already uses numbers, so right now the two don't match each other.
|
|
||
| ## Before Starting | ||
|
|
||
| > **Important:** The ADR and the [Roster Statistics](#roster-statistics) page are driven by **different** files. The ADR reads billet groups by index from `client/app/adr/page.jsx`; Roster Statistics reads billet IDs from `BilletBank.jsx`. A new billet or unit usually needs to be added in **both** places (and, for the regiment chart, a matching color). See [Files to Update](#files-to-update) for the full list. |
There was a problem hiding this comment.
This is a bit strong for a single new billet. If it's going into a group that's already in the ADR, the ADR picks it up live from the API on its own, so only Statistics (BilletBank) needs touching. It's a brand new group or category that needs both files. The note at the bottom (L229) basically says this already, so worth lining the two up.
| ### Add New Billet in Existing Category | ||
|
|
||
| To add a new billet to an existing category, you need to update the `BilletBank.jsx` file located in `cavapps/client/app/reusableModules`. | ||
| To add a new billet to an existing category, you need to update the `page.jsx` file located in `cavapps/client/app/adr`. |
There was a problem hiding this comment.
There's no cavapps/ folder in the repo, the path's just client/app/adr. Same prefix is at L192, L303 and L329 too. The Files to Update list lower down already uses the right client/app/... form, so matching that everywhere does it.
|
|
||
| For further documentation on Next.js, visit https://nextjs.org/docs | ||
|
|
||
| ## Before Starting |
There was a problem hiding this comment.
New section, but it didn't make it into the Table of Contents up top where every other ## is listed. Since the whole thing is one callout, folding it into "Updating the ADR" as a note might be simpler than a standalone heading. Either way works as long as the TOC matches.
|
|
||
| Unlike the ADR, Roster Statistics reads its billet IDs from the `BilletBank.jsx` file located in `cavapps/client/app/reusableModules`. | ||
|
|
||
| ### Add New Billet in Existing Category |
There was a problem hiding this comment.
The second "Add New Billet…" and "Add New Category" headings (L305, L327) are duplicates of the ones up top, so GitHub gives them -1 anchors that nothing links to. Tagging them like "Add New Category (Roster Statistics)" makes them linkable and keeps markdownlint quiet.
What this does