Skip to content

Document and test dimension mapping utilities#1829

Merged
axelboc merged 1 commit into
mainfrom
dimmapping-docs
Jun 23, 2025
Merged

Document and test dimension mapping utilities#1829
axelboc merged 1 commit into
mainfrom
dimmapping-docs

Conversation

@axelboc

@axelboc axelboc commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

Final PR about moving the dimension mapper and related utilities to the lib.

  • Extract and expose a non-memoised utility from useSlicedDimsAndMapping: getSlicedDimsAndMapping.
  • Document initDimMapping, getSliceSelection, getSlicedDimsAndMapping (and useSlicedDimsAndMapping)
  • Unit test getSliceSelection and getSlicedDimsAndMapping (I already wrote tests for initDimMapping in a previous PR)

import { type Axis, type DimensionMapping } from '@h5web/shared/vis-models';

import styles from './DimensionMapper.module.css';
import { type DimensionMapping } from './models';

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

I've moved the DimensionMapping type to the shared package. It seemed more logical and it was lonely in a file all by itself... 😁


interface Props extends Omit<HTMLAttributes<HTMLDivElement>, 'onChange'> {
dims: number[];
dimHints?: AxisMapping<string>;

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

AxisMapping was created as a NeXus concept (it's in nexus-models.ts in the shared package). Now that the DimensionMapper is exposed, I prefer to remove this subtle coupling with NeXus. It makes the type of the dimHints prop more explicit at the same time.

@axelboc axelboc requested a review from loichuder June 23, 2025 12:35
@axelboc axelboc merged commit 928618d into main Jun 23, 2025
9 checks passed
@axelboc axelboc deleted the dimmapping-docs branch June 23, 2025 14:51
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.

2 participants