Skip to content

Fixes two issues in the Scene UI & adds Storybook test cases for the Scene UI#6564

Merged
Exairnous merged 2 commits intoHubs-Foundation:masterfrom
DougReeder:storybook-tips
Nov 4, 2025
Merged

Fixes two issues in the Scene UI & adds Storybook test cases for the Scene UI#6564
Exairnous merged 2 commits intoHubs-Foundation:masterfrom
DougReeder:storybook-tips

Conversation

@DougReeder
Copy link
Copy Markdown
Member

@DougReeder DougReeder commented Aug 22, 2025

What?

Fixes #6568
Ensures, on an unavailable scene, the text "This scene is no longer available" is always legible
Adds Storybook test cases for scene-ui.js
Also rounds out a few other Storybook stories.

Why?

  1. The stories didn't exist yet, and production code has been changed to use the Web Share API.
  2. The other stories didn't fully test what they were supposed to.

How to test

  1. Run npm run storybook
  2. Surf to http://localhost:6006/?path=/story/scene-ui--base
  3. Examine each of the scene-ui stories (Show unavailable has white text on a white background, as Storybook could use better style configuration).
  4. Navigate to the ChatSidebar — Base story
  5. Vary the window width, to see how line breaking behaves.
  6. Navigate to Tip — Tooltips
  7. In the Controls section at the bottom, select each test case and inspect the UI.
  8. In a Hubs instance with this PR, navigate to the scene page for a scene with attributions. Observe that attributions are displayed.
  9. In a Hubs instance with this PR, navigate to a scene which is no longer available. Observe that the text "This scene is no longer available" is legible.

Limitations

None of the Mobile test cases for Tooltip worked before, and they still don't. The UI in the actual app is fine. It's not clear why Storybook gives bad results here.

Alternatives considered

If the Mobile test cases for Tooltip were included, it would be natural for a dev to conclude the actual UI is wrong. That would waste dev time on something that isn't broken in the actual app.

Open questions

How could Storybook be better configured, or the Tooltip story better written?

Additional details or related context

This depends on Matt's Storybook fixes. #6563

@DougReeder DougReeder force-pushed the storybook-tips branch 2 times, most recently from 4d26050 to ec9f73f Compare August 27, 2025 03:52
@DougReeder DougReeder force-pushed the storybook-tips branch 2 times, most recently from 630e062 to 46f65e0 Compare October 28, 2025 15:43
@DougReeder
Copy link
Copy Markdown
Member Author

With the SCSS change:
image

@DougReeder
Copy link
Copy Markdown
Member Author

Fixes #6568 and the story "Scene Attributions" shows that. See the image above.

Also fixes story "Show Unavailable". The text was there, but white text on a white background was not visible. The change to scene-ui.js ensures the text is visible regardless of whether the screen shot is light or dark.

Doesn't fix #6567 and the stories "Scene Allow Remixing" and "Owner Project Id" reflect that. Also, it was not clear how to set the app feature "Enable Scene Editor" on, for Storybook.

@DougReeder DougReeder changed the title Adds Storybook test cases for scene-ui.js Fixes two issues in the Scene UI & adds Storybook test cases for the Scene UI Oct 28, 2025
@DougReeder DougReeder requested a review from Exairnous October 29, 2025 13:02
Why: Stories didn't exist yet, and they would be confusing without the fixes
@matthargett
Copy link
Copy Markdown
Contributor

some general comments you can take or leave :)

  • I think I see two more variants you coudl add to mirror issue Scene page does not display attributions #6568 : one that passes a parentScene (to validate the remix attribution path) and one that feeds the legacy sceneAttributions.extras field.
  • For the create-room story, consider stubbing createAndRedirectToNewHub so clicking the button doesn’t try to POST against a real backend during local Storybook runs. needing to have a real local backend running just to do a visual regression test is a notable barrier
  • src/react-components/scene-ui.stories.js:24 – does the Remix variant ever render its CTA? it looks like the component sits behind IfFeature('enable_spoke'), and Storybook boots with that flag off. The story shows the same UI as Base, so it looks like it can’t regress the scenario it’s meant to cover
  • Remix link is emitted as /spoke/projects/new?sceneId=undefined since the story omits sceneId. that would seem to not represent the primary behavior, right? Supplying a realistic id (and, ideally, plumbing it through Storybook args) should fix the href

@DougReeder
Copy link
Copy Markdown
Member Author

  • stubbing createAndRedirectToNewHub so clicking the button doesn’t try to POST

Is there documentation on stubbing?

does the Remix variant ever render its CTA? it looks like the component sits behind IfFeature('enable_spoke'), and Storybook boots with that flag off

Is there a way to set IfFeature('enable_spoke') on for some stories and off for others?

/>
);

export const ParentScene = () => (
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 seems like it's showing a child/remixed scene, so I think this sub-story should be named "Child Scene" or "Has Parent Scene"?

Why: the attributions for a remixed scene were neither fully tested nor fully working
@DougReeder DougReeder marked this pull request as ready for review November 4, 2025 18:32
Copy link
Copy Markdown
Member

@Exairnous Exairnous left a comment

Choose a reason for hiding this comment

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

With the ParentScene sub-story renamed to RemixedScene, everything looks good to me. Thanks.

Edit: Merged after discussion in the dev meetup.

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.

Scene page does not display attributions

3 participants