Skip to content

Arranger Charts Docs#1083

Open
samrichca wants to merge 7 commits into
mainfrom
feat/docs-charts
Open

Arranger Charts Docs#1083
samrichca wants to merge 7 commits into
mainfrom
feat/docs-charts

Conversation

@samrichca

Copy link
Copy Markdown
Member

Summary

Issues

  • #(issue number)
  • #(issue number)

Description of Changes

Readiness Checklist

  • Self Review
    • I have performed a self review of code
    • I have run the application locally and manually tested the feature
    • I have checked all updates to correct typos and misspellings
  • Formatting
    • Code follows the project style guide
    • Autmated code formatters (ie. Prettier) have been run
  • Local Testing
    • Successfully built all packages locally
    • Successfully ran all test suites, all unit and integration tests pass
  • Updated Tests
    • Unit and integration tests have been added that describe the bug that was fixed or the features that were added
  • Documentation
    • All new environment variables added to .env.schema file and documented in the README
    • All changes to server HTTP endpoints have open-api documentation
    • All new functions exported from their module have TSDoc comment documentation

Comment thread code_of_conduct.md Outdated

@justincorrigible justincorrigible Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good call adding this! however, it belongs at the Overture-stack org level, so we can relocate it there. I'll put that in the roadmap, and we can leave it here for now 👍

@justincorrigible justincorrigible Jul 13, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

scratch that. this file can be removed, we already point at a code of conduct in the overture docs site from the CONTRIBUTING.md in this repo


### BarChart

Renders a horizontal bar chart for aggregation data.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If introducing a visual component please include a screenshot of the component.

# Overview

**Arranger Charts** is a plugin for Arranger UIs for adding interactive aggregated data charts to React applications. It currently supports Bar and Sunburst charts. Arranger Charts also uses a React context provider to request data from Arranger Server for all charts on the page. Interacting with the charts can update the filters for data requested from Arranger Server.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please include a screen shot of the charts within a discovery page

Comment thread modules/charts/README.md Outdated
Comment thread docs/charts/02-Setup.md

## Developer Setup

This guide will walk you through setting up a complete development environment for Arranger Charts, including its complementary services.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This statement seems out of place given the context of the page

Comment thread docs/charts/02-Setup.md
npm i @overture-stack/arranger-charts @overture-stack/arranger-components
```

### Quick Start

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not really a quick start more of an implementation guide which is great, should think about how we can flesh this out a bit more, "building a discovery page"

samrichca and others added 2 commits July 10, 2026 14:12
Co-authored-by: Mitchell Shiell <59712867+MitchellShiell@users.noreply.github.com>
Comment on lines +15 to +17
| `debugMode` | `boolean` | No | Enable verbose logging for development |
| `disableIncludeMissing` | `boolean` | No | Hide properties with "No Data" |
| `loadingDelay` | `number` | No | Delay network results by milliseconds |

@justincorrigible justincorrigible Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

while debugMode and loadingDelay are correctly marked as optional, the type actually requires them in Provider.tsx:22-26. they need to be made optional there

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry, should I do that on this PR or make a new issue/PR?

| `components` | `object` | No | Custom fallback components |
| `components.EmptyData` | `ReactComponent` | No | Custom empty state component |
| `components.ErrorData` | `ReactComponent` | No | Custom error component |
| `components.Loader` | `ReactComponent` | No | Custom loading component |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think components.TooltipComp could also go here, but maybe not worth adding? 🤔
not sure what the right call is here.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TooltipComp never gets used :/

Comment thread docs/charts/03-Guides/01-Usage.md Outdated
| `handlers.onClick` | `function` | No | Callback when clicking a bar segment |
| `maxBars` | `number` | Yes | Maximum number of bars to display |
| `ranges` | `Range[]` | No | For numeric fields, specify value ranges. A `Range` is `{ key: string, from: number, to: number }`. |
| `theme` | `object` | No | Event handlers |

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the description here seems like missed copy/pasta, and this is a required property at BarChart.tsx:23-30

Comment thread docs/charts/03-Guides/01-Usage.md Outdated

const chartFilters = useMemo(
() => ({
[fieldName]: chartFilter(fieldName, sqon, setSQON),

@justincorrigible justincorrigible Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

chartFilter is not shown as imported... if this is something consumers are expected to implement themselves, a comment here may help identify that

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I decided to remove this example because there's a fair bit of additional code involved and it's really specific to OHCRN.

Comment thread docs/charts/03-Guides/01-Usage.md Outdated
<ChartsThemeProvider>
<BarChart
fieldName={fieldName}
maxBars={maxBars}

@justincorrigible justincorrigible Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

maxBars wasn't defined anywhere. this could be shown as commented with a comment if BarChart has a default

Comment thread docs/charts/03-Guides/01-Usage.md Outdated
[sqon, setSQON],
);

return (

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the other examples in this doc are wrapped in <ArrangerDataProvider>
may be worth adding it here too as it's needed and for consistency

Comment thread docs/charts/03-Guides/01-Usage.md Outdated
| Name | Type | Required | Description |
| ------------------ | ---------- | -------- | ----------------------------------------------- |
| `fieldName` | `string` | Yes | GraphQL field name to visualize |
| `maxSegments` | `number` | Yes | Custom fallback components |

@justincorrigible justincorrigible Jul 10, 2026

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

the description doesn't seem accurate. it used to say "Maximum number of segments to display".
copy pasta from components @ ChartsThemeProvider?

Comment thread modules/charts/README.md Outdated
# Arranger Charts

A React chart library for visualizing Arranger aggregation.
!TODO Rename to README.md

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

probably not needed anymore?

Comment thread modules/charts/README.md Outdated
- `Loader`: Custom loading component
- `ErrorData`: Custom error component
- `EmptyData`: Custom empty state component
- Node 22 or above

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

line 80 states v24+ (which i like)

Comment thread docs/charts/03-Guides/02-Development.md Outdated
@@ -0,0 +1,46 @@
# Development

This guide covers how to develop or contribute to Arranger Charts. For using Arranger Charts in a React application, see !TODO LINK TO GUIDES/USAGE.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this link can be added now 👍

Comment on lines +19 to +26
```bash
# in your local Arranger folder
cd modules/charts
npm link
cd ../..
cd modules/components
npm link
```

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

note to self @justincorrigible: make this a batch/make command

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants