Skip to content
This repository has been archived by the owner on Oct 16, 2023. It is now read-only.

refactor: Update header components #32

Merged
merged 13 commits into from
Jul 14, 2022

Conversation

kjohnson
Copy link
Contributor

@kjohnson kjohnson commented Jul 12, 2022

Resolves #31

Description

This PR updates the Form Builder Header Components to reflect the updated design (at least for the components with corresponding features that have been built out).

NOTE: There are some refactoring that this will still need at some point, most notably cleaning up the relationship between icons and buttons, reusable styling, etc. This is focused on getting the components ready for polish.

Visuals

develop
Screen Shot 2022-07-13 at 16 32 57
refactor/update-builder-header-31
Screen Shot 2022-07-13 at 16 32 38

Pre-review Checklist

  • Acceptance criteria satisfied and marked in related issue
  • Relevant @unreleased tags included in DocBlocks
  • Includes unit tests
  • Reviewed by the designer (if follows a design)
  • Self Review of code and UX completed

@github-actions
Copy link

A build of this branch is available at https://impress-org.github.io/givewp-next-gen/refactor/update-builder-header-31

@kjohnson kjohnson marked this pull request as ready for review July 12, 2022 19:53
@DevinWalker
Copy link
Member

Hey @kjohnson nice job here - please see my video review here:

https://www.loom.com/share/18f0a0865484402f9420035b55517c62

Highlights:

  1. Cursor on hover of clickable elements
  2. Start discussion about "Publish" being a primary buttons style, not secondary. Also adding some hover transition effects like Gutenberg for polish.
  3. UX/UI - We need to decide how to make the fact that the title is editable on click more evident

cc @jdghinson for items 2 and 3 above.

@kjohnson
Copy link
Contributor Author

Per the call today, I'll update the Publish button to reflect the change to a "primary" button style.

@kjohnson kjohnson marked this pull request as draft July 13, 2022 19:03
@kjohnson kjohnson marked this pull request as draft July 13, 2022 19:03
@kjohnson kjohnson marked this pull request as ready for review July 13, 2022 20:32
toggleSecondarySidebar={toggleSecondarySidebar}
showSidebar={showSidebar}
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@jonwaldstein This will probably need to be refactored to a context hook for builder state. I'm looking into that for a later change.Thoughts?

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah i'm thinking there will be an application provider for managing this kind of stuff, what you have is fine for now though.

@kjohnson
Copy link
Contributor Author

@jonwaldstein flex-basis looks cool - hadn't seen that before.

…r. organize folder structure a little more
Copy link
Contributor

@jonwaldstein jonwaldstein left a comment

Choose a reason for hiding this comment

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

@kjohnson I made a few updates here:

  1. Header css updates to make the 3 parts more consistent widths.
  2. Separated the Header component into presentational and container components. I like to do this as much as possible to isolate components that can truly be pure components, where others are functional.
  3. Some folder structure updates, moved some things out of the "components" directory, created a "common" directory, and a few root entry points so we can get components & common things easier.

We're getting to a point where the application is growing and it's going to be really important to keep things organized the best we can. Speaking from experience, these react apps can take a shape of their own if we're not careful 😁.

Otherwise, great job!

@kjohnson kjohnson merged commit c0c5cae into develop Jul 14, 2022
@github-actions
Copy link

A build of this branch is available at https://impress-org.github.io/givewp-next-gen/develop

@jonwaldstein jonwaldstein deleted the refactor/update-builder-header-31 branch July 15, 2022 12:24
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update Header Component to reflect Figma design
3 participants