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.

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

#2557 Part 1 - Show blueprints in sidebar #2590

Merged
merged 30 commits into from
Feb 17, 2022

Conversation

BLoe
Copy link
Collaborator

@BLoe BLoe commented Feb 4, 2022

Starts the work for #2557.

Apologies for the size of this PR 😅

The main changes are:

  • Group the sidebar items by recipe where possible, add expand/collapse functionality to show extensions
  • Add activeExtension stuff to redux
  • Add a new EditorTabLayout component that is domain agnostic, it just knows how to render a tab layout with action buttons in the top right
  • Use this to implement the basic RecipePane for selected recipes
    • All the dependencies here are stubbed and there's just text in the 3 tabs (Edit, Options, Logs)

@twschiller
Copy link
Contributor

@BLoe This looks like a good start. What would be involved in feature flagging the blueprint work? This kind of work has the potential to become a long-lived branch

@BLoe
Copy link
Collaborator Author

BLoe commented Feb 7, 2022

I just pushed a commit that puts the blueprint grouping in the sidebar behind the beta feature flag. It'll be a little bit more complicated to manage the tabs and stuff but should be possible.

@BLoe BLoe requested a review from twschiller February 15, 2022 05:18
@fregante
Copy link
Collaborator

Can you give me a few short instructions on how to see this new UI? Do I have to install one or more blueprints from the new blueprints page?

@BLoe
Copy link
Collaborator Author

BLoe commented Feb 15, 2022

Can you give me a few short instructions on how to see this new UI? Do I have to install one or more blueprints from the new blueprints page?

Activate any blueprint from the marketplace or my-blueprints page, then open the page editor. You also need the page-editor-beta feature flag, so you'll have to ask Todd to enable that for you if you don't have it.

@BLoe BLoe marked this pull request as ready for review February 15, 2022 17:08
@BLoe
Copy link
Collaborator Author

BLoe commented Feb 15, 2022

@fregante @BALEHOK I think we should try to get this merged in despite the intermediate state of things, due to the fact that all 3 of us are making page editor layout/css changes right now. Everything here should be contained behind the page-editor-beta feature flag.

I'm open to really thorough thoughts/feedback/whatever from anyone, since this affects everything in the page editor.

We can also try and huddle up on slack or otherwise have a discussion about things if anyone thinks that would be helpful.

index
) => (
<Button
key={index} // Action buttons shouldn't normally be changing order
Copy link
Contributor

Choose a reason for hiding this comment

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

index is ok, caption will do as well I believe.

@BLoe BLoe requested a review from BALEHOK February 16, 2022 23:45
@BLoe
Copy link
Collaborator Author

BLoe commented Feb 16, 2022

@twschiller @fregante @BALEHOK

I added the tests I wanted to for the grouping logic. I think we should try and get this merged now, but would welcome any feedback from anyone if you guys feel like reviewing it.

@BLoe BLoe merged commit c7c0a76 into main Feb 17, 2022
@BLoe BLoe deleted the feature/2557-1-show-blueprints-in-sidebar branch February 17, 2022 15:55
onClick: saveRecipe,
caption: "Save",
disabled: isSavingRecipe,
icon: faSave,
Copy link
Collaborator

Choose a reason for hiding this comment

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

It seems that this is reimplementing the ElementWizard’s nav from scratch (duplicating it) and is incompatible with my PR because the styles are no longer global:

Probable solution:

  • move nav to its own component and reuse it

Copy link
Collaborator

Choose a reason for hiding this comment

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

Issue demo:

bad

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.

4 participants