UMN-LATIS / elevator-ui

UI for Elevator, a flexible digital asset repository
2 stars 0 forks source link

make custom header mode configurable #269

Closed cmcfadden closed 4 months ago

cmcfadden commented 4 months ago

knocked this out during a meeting, but happy to have @jxjj refactor if it uglifies the codebase too much.

jxjj commented 4 months ago

Super cool! Looks great!

Two suggestions:

  1. The instance setting says "only on Home Page", but I think it applies to both home and content pages. Maybe clarify the setting name?

  2. With those magic numbers (0,1,2) it's hard to tell which mode is which. Creating named constants would make it a bit easier to read.

Something like:

// constants/constants.ts

export const CUSTOM_HEADER_MODES = {
  NEVER: 0,
  ALWAYS: 1,
  PAGES_ONLY: 2,
} as const;

If you want, you could more explicitly type the customHeaderMode field to get autocomplete magic and type checking.

// types/index.ts

// this magic bit means permit any values of the CUSTOM_HEADER_MODES object
type CustomHeaderMode = typeof CUSTOM_HEADER_MODES[keyof typeof CUSTOM_HEADER_MODES];

interface ... {
  customHeaderMode: CustomHeaderMode // 0 | 1 |  2
}