Skip to content

Comments

alphafold--first-test-url#1579

Merged
aurreco-uga merged 2 commits intov1.3.6-patchfrom
alphafold--first-test-url
Jan 13, 2026
Merged

alphafold--first-test-url#1579
aurreco-uga merged 2 commits intov1.3.6-patchfrom
alphafold--first-test-url

Conversation

@aurreco-uga
Copy link
Member

No description provided.

@aurreco-uga aurreco-uga requested a review from bobular January 7, 2026 14:26
@aurreco-uga
Copy link
Member Author

Screenshot 2025-12-25 at 12 55 40 PM

Copy link
Member

@bobular bobular left a comment

Choose a reason for hiding this comment

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

Hi @aurreco-uga - I figured out the problem and solution but got Claude to do the typing below.

Problem

The error messages in the AlphaFold section are rendering outside the CollapsibleSection component. Looking at the screenshot provided, the pink error message appears as a standalone element rather than being contained within the expandable/collapsible "AlphaFold" section.

Current Behavior

The code has several early returns that render error states:

  • No data URL (lines 81-92)
  • Loading state (lines 95-106)
  • Invalid/404 URL (lines 109-145)

These all return plain <div> elements that are not wrapped in the CollapsibleSection structure. Only when dataUrlStatus === 'valid' does it render <BlockRecordAttributeSection {...props} />, which provides the proper collapsible section wrapper.

Additional Issue

The error messages may be rendering regardless of whether the section is collapsed or expanded. The error states should only be visible when the section is open.

Proposed Solution

Create a wrapper component within the same file that mimics what BlockRecordAttributeSection does - it should:

  1. Set up the CollapsibleSection structure
  2. Accept children as a prop to render custom content inside the section
  3. Respect the props.isCollapsed state

Example Implementation

  // Add this wrapper component in AlphaFoldAttributeSection.tsx
  function AlphaFoldErrorWrapper({
    children,
    attribute: { name, displayName, help },
    isCollapsed,
    onCollapsedChange,
    title
  }: Props & { children: React.ReactNode }) {
    const headerContent = title ?? (
      <DefaultSectionTitle displayName={displayName} help={help} />
    );

    return (
      <CollapsibleSection
        id={name}
        className="wdk-RecordAttributeSection"
        headerContent={headerContent}
        isCollapsed={isCollapsed}
        onCollapsedChange={onCollapsedChange}
      >
        <div className="wdk-RecordAttributeSectionContent">
          {children}
        </div>
      </CollapsibleSection>
    );
  }

Note: You'll need to import CollapsibleSection and DefaultSectionTitle from the appropriate WDK locations.

Then Update the Early Returns

Replace the plain <div> returns with the wrapper:

// No data URL case (lines 81-92)
if (!hasDataUrl) {
  return (
    <AlphaFoldErrorWrapper {...props}>
      <div className="wdk-RecordAttributeSectionItem" style={{ padding: '1em' }}>
        <p>
          <em>AlphaFold structure prediction not available for this gene.</em>
        </p>
      </div>
    </AlphaFoldErrorWrapper>
  );
}

// + do similar for:
// Loading state (lines 95-106)
// Invalid URL case (lines 109-145)

Benefits

  1. Consistent UI: All states (error, loading, valid) appear within the same collapsible section structure
  2. Proper Collapsing: Error messages will only be visible when the section is expanded
  3. Cleaner Code: Reuses the existing section wrapper pattern
  4. Better UX: Users see the AlphaFold section header and can choose to expand/collapse it, regardless of the content state

Next Steps

  1. Examine BlockRecordAttributeSection implementation to determine the exact CollapsibleSection imports and props needed
  2. Implement the AlphaFoldErrorWrapper component
  3. Update all early return statements to use the wrapper
  4. Test that the section collapses/expands properly in all states
  5. Include testing of expand/collapse from left-hand navigation panel

@bobular
Copy link
Member

bobular commented Jan 8, 2026

btw I did consider alternative solutions. We could have enhanced BlockRecordAttributeSection to take error message props and it could have displayed those, but that felt like overkill for a single use case. The wrapper approach keeps the validation logic localized to the AlphaFold component without adding complexity to a shared component used throughout the codebase.

@aurreco-uga
Copy link
Member Author

aurreco-uga commented Jan 9, 2026

omar approved the current proposal, UI wise.. how about merging it into the patch branch and we work on your proposal for main? i could see other sections with similar issues and it would be good to handle all the same...

@bobular
Copy link
Member

bobular commented Jan 9, 2026

I'd need to test it on a dev site first. Do you have one? It wasn't simple for me to put up a local dev server or a dev site due to the yarn3/yarn4 divergence.

@aurreco-uga
Copy link
Member Author

i have my implement on https://aurreco.plasmodb.org/plasmo.aurreco/app/record/gene/PfDd2_130048800
(68g site with patch branch for monorepo plus updates for alphafold)

or are you meaning to test your proposal on a master site? i thought we could do that later

@bobular
Copy link
Member

bobular commented Jan 9, 2026

Thanks.

I don't like the way it breaks the page mechanics. Once the section has been expanded there is no way to hide the warning message by collapsing it again. Even via the left-hand nav menu.

I prefer to implement* my suggestion (I can do it if you prefer) and then re-implement something more general later when we have identified the wider need.

ETA: * and cherry pick into main

@bobular
Copy link
Member

bobular commented Jan 9, 2026

Hi @aurreco-uga - I demoed and discussed with Steve in scrum and we're fine with this being merged as-is to the legacy branch(es). If we need to revisit this on the feature site/main (Kathryn's latest message suggests it may be less of a problem when the data is less stale) then we have this PR to work from.

@aurreco-uga
Copy link
Member Author

@aurreco-uga aurreco-uga merged commit d0f3b3a into v1.3.6-patch Jan 13, 2026
1 check passed
@aurreco-uga aurreco-uga deleted the alphafold--first-test-url branch January 13, 2026 08:52
aurreco-uga added a commit that referenced this pull request Jan 13, 2026
* alphafold--first-test-url

* request to show message within collapsible section

(cherry picked from commit d0f3b3a)
// Handle missing data URL
if (!hasDataUrl) {
return (
<div
Copy link
Member

Choose a reason for hiding this comment

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

Just wondering why you didn't wrap this (and the next "early return" a few lines down) in the AlphaFoldErrorWrapper?

@aurreco-uga
Copy link
Member Author

we havent seen instances of those.. probably i was in a hurry and focused on what was needed..

@bobular
Copy link
Member

bobular commented Jan 14, 2026

we havent seen instances of those.. probably i was in a hurry and focused on what was needed..

Sure - no worries!

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