Skip to content

#9780 - Save the polymer type switcher (RNA/DNA/PEP) to browser cache#10148

Open
mariam-khutuashvili wants to merge 2 commits into
masterfrom
9780-save-the-polymer-type-switcher-rna-dna-pep-to-browser-cache
Open

#9780 - Save the polymer type switcher (RNA/DNA/PEP) to browser cache#10148
mariam-khutuashvili wants to merge 2 commits into
masterfrom
9780-save-the-polymer-type-switcher-rna-dna-pep-to-browser-cache

Conversation

@mariam-khutuashvili
Copy link
Copy Markdown
Collaborator

@mariam-khutuashvili mariam-khutuashvili commented Jun 2, 2026

How the feature works? / How did you fix the issue?

(Screenshots, videos, or GIFs, if applicable)

Description

The Macromolecules polymer-type switcher (RNA / DNA / PEP) now remembers the last-selected type in the browser
cache (localStorage) and restores it when Macromolecules mode is opened, instead of always resetting to RNA.
Defaults to RNA when nothing is stored.

Requirements covered

  • Saves the last switcher option to browser cache, persisted on every change path: clicking the switcher and the
    Ctrl+Alt+R/D/P hotkeys (both funnel through the single changeSequenceTypeEnterMode event).
  • On opening Macromolecules mode, the polymer type is restored from cache.
  • Defaults to RNA when no value is stored, or when the stored value is invalid.
  • The restored type also drives the default monomer-library tab (PEP → Peptides tab), including in snake/flex modes
    where the switcher is not visible — the switcher component is always mounted, so the restore-on-open dispatch
    flows through the existing type→tab mapping.

Changes

  • constants.ts — added SEQUENCE_TYPE_STORAGE_KEY.
  • helpers/sequenceTypeStorage.ts (new) — getPersistedSequenceType() (reads and validates against SequenceType,
    falls back to RNA) and persistSequenceType(), following the existing localStorageWrapper convention.
  • helpers/sequenceTypeStorage.test.ts (new) — unit tests: default RNA, persist/restore round-trip, invalid value →
    RNA.
  • SequenceTypeGroupButton.tsx — persist the type on change, and restore from cache on mount instead of hardcoding
    RNA.

Check list

  • unit-tests written
  • e2e-tests written
  • documentation updated
  • PR name follows the pattern #1234 – issue name
  • branch name doesn't contain '#'
  • PR is linked with the issue
  • base branch (master or release/xx) is correct
  • task status changed to "Code review"
  • reviewers are notified about the pull request

Persist the selected polymer type to localStorage whenever it changes (switcher click or Ctrl+Alt+R/D/P) and restore it when Macromolecules mode opens, defaulting to RNA when nothing is cached. The restored type also drives the default monomer-library tab.
Comment thread packages/ketcher-macromolecules/src/helpers/sequenceTypeStorage.ts
Comment thread packages/ketcher-macromolecules/src/helpers/sequenceTypeStorage.ts Outdated
@claude
Copy link
Copy Markdown
Contributor

claude Bot commented Jun 2, 2026

Review summary

Small, focused change with a clean helper and good test coverage of the validation path. A few things worth addressing before merge — all noted as inline comments:

  • Handler leak (major). The new persistSequenceType call lives inside a changeSequenceTypeEnterMode listener that the effect cleanup doesn't remove. Pre-existing, but amplified by the new side effect — easy to fix in this PR.
  • Unguarded JSON.parse in the helper (minor). localStorageWrapper.getItem will throw on a malformed stored value and crash the mount effect before isSequenceType runs. Worth a try/catch and a test for the malformed-JSON case.
  • Wording nit. "browser cache" in the inline comment → "localStorage" to match the surrounding codebase.

Security review found nothing of concern (strict enum equality on the deserialized value, no DOM/HTML/navigation sink). No public-API or changelog doc updates are missing — the analogous PRESET_PHOSPHATE_FILTER_STORAGE_KEY feature shipped the same way.

…leaked handler

- Guard getPersistedSequenceType against malformed JSON (JSON.parse can throw); log via KetcherLogger and fall back to RNA, with a test.
- Reword comment 'browser cache' -> 'localStorage' to match the codebase.
- Lift the changeSequenceTypeEnterMode handler to a named const and remove it in the effect cleanup so it no longer leaks on re-run/StrictMode.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Save the polymer type switcher (RNA/DNA/PEP) to browser cache

2 participants