Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

#8759: always show mod variable data panel in the Page Editor #8760

Merged
merged 5 commits into from
Jul 8, 2024
Merged

#8759: always show mod variable data panel in the Page Editor #8760

merged 5 commits into from
Jul 8, 2024

Conversation

twschiller
Copy link
Contributor

@twschiller twschiller commented Jul 6, 2024 •
edited
Loading

What does this PR do?

  • Closes Always show mod variables Data Panel tab #8759
  • Always show the mod variable data panel in the Page Editor
  • Rename tab name in data panel from Page State to "Mod Variables"
  • Cleans up some outdated/ambiguous tab and React component names

Coordination

  • Changing the tab name/event keys might impact @fungairino's POM work

Checklist

Leave all that are relevant and check off as completed

For more information on our expectations for the PR process, see the
code review principles doc

@twschiller twschiller self-assigned this Jul 6, 2024
@twschiller twschiller added the user experience Improve the user experience (UX) label Jul 6, 2024
@twschiller twschiller added this to the 2.0.5 milestone Jul 6, 2024
@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 6, 2024 •
edited
Loading

Playwright test results

passed  81 passed
flaky  3 flaky
skipped  2 skipped

Details

report  Open report ↗︎
stats  86 tests across 30 suites
duration  14 minutes, 32 seconds
commit  2d0817a

Flaky tests

edge › tests/extensionConsole/activation.spec.ts › can activate a mod with built-in integration
edge › tests/pageEditor/saveMod.spec.ts › can save a standalone trigger mod
edge › tests/regressions/welcomeStarterBricks.spec.ts › #8740: can view the starter mods on the pixiebrix.com/welcome page

Skipped tests

chrome › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options
edge › tests/runtime/googleSheetsIntegration.spec.ts › can activate a google spreadsheet mod with config options

twschiller
twschiller commented Jul 6, 2024
View reviewed changes
src/testUtils/factories/pageEditorFactories.ts
@@ -69,7 +69,7 @@ const internalFormStateFactory = define<InternalFormStateOverride>({
apiVersion: "v3" as ApiVersion,
uuid: uuidSequence,
installed: true,
optionsArgs: {} as OptionsArgs,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fix cooky-cutter warning about object re-use

@codecov Codecov
Copy link

codecov bot commented Jul 6, 2024 •
edited
Loading

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 74.24%. Comparing base (8318d74) to head (2d0817a).
Report is 13 commits behind head on main.

Additional details and impacted files 
@@            Coverage Diff             @@
##             main    #8760      +/-   ##
==========================================
- Coverage   74.24%   74.24%   -0.01%     
==========================================
  Files        1332     1332              
  Lines       40817    40820       +3     
  Branches     7634     7631       -3     
==========================================
  Hits        30306    30306              
- Misses      10511    10514       +3

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

twschiller
twschiller commented Jul 6, 2024
View reviewed changes
src/pageEditor/tabs/editTab/EditTab.tsx
@@ -159,7 +159,7 @@ const EditTab: React.FC<{
<div className={styles.dataPanelWrapper}>
<div className={styles.dataPanel}>
{activeNodeId === FOUNDATION_NODE_ID ? (
<FoundationDataPanel />
<StarterBrickDataPanel />
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Renamed to use current terminology

Copy link
Collaborator

Choose a reason for hiding this comment

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

Should we also rename FOUNDATION_NODE_ID?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I hadn't renamed in this PR because there's some other uses of foundation in reference to the foundation node type as well. I think we should do as a separate PR

@github-actions GitHub Actions
Copy link

github-actions bot commented Jul 8, 2024

No loom links were found in the first post. Please add one there if you'd like to it to appear on Slack.

Do not edit this comment manually.

@twschiller twschiller merged commit 359812b into main Jul 8, 2024
25 checks passed
@twschiller twschiller deleted the feature/data-panel-mod-variables branch July 8, 2024 15:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@grahamlangford grahamlangford grahamlangford approved these changes

@mthek10 mthek10 Awaiting requested review from mthek10

@brittanyjoiner15 brittanyjoiner15 Awaiting requested review from brittanyjoiner15

@fungairino fungairino Awaiting requested review from fungairino

Assignees

@twschiller twschiller

Labels
developer experience page editor user experience Improve the user experience (UX)
Milestone
2.0.5
Development

Successfully merging this pull request may close these issues.

Always show mod variables Data Panel tab
2 participants
@twschiller @grahamlangford