Skip to content
This repository has been archived by the owner on Mar 13, 2024. It is now read-only.

Feature/header update #132

Open
wants to merge 7 commits into
base: develop
Choose a base branch
from
Open

Feature/header update #132

wants to merge 7 commits into from

Conversation

con322
Copy link
Contributor

@con322 con322 commented Jan 31, 2019

Proposed Changes

  • Add ability to place header behind navbar
  • Add full height option to size dropdown
  • Add optional bullet block
  • Add optional second button

@con322 con322 requested a review from jaymcp January 31, 2019 10:58
@jaymcp jaymcp added this to the v1.2.0 milestone Feb 4, 2019
jaymcp
jaymcp previously requested changes Feb 4, 2019
View reviewed changes
Copy link
Member

@jaymcp jaymcp left a comment

Choose a reason for hiding this comment

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

Functionally, these changes work well, though there are some potential issues with the source.
There is a margin bottom on the bullet list on the front-end that needs removing:

header.php Outdated Show resolved Hide resolved
header.php Outdated Show resolved Hide resolved
includes/blocks/header/render.php Outdated Show resolved Hide resolved
includes/blocks/header/render.php Outdated Show resolved Hide resolved
includes/blocks/header/render.php Outdated Show resolved Hide resolved
includes/blocks/header/render.php Outdated Show resolved Hide resolved
src/scripts/app.js Outdated Show resolved Hide resolved
@jaymcp jaymcp dismissed their stale review February 4, 2019 15:26

Requested changes were made

jaymcp
jaymcp approved these changes Feb 4, 2019
View reviewed changes
Copy link
Member

@jaymcp jaymcp left a comment

Choose a reason for hiding this comment

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

LGTM

@jaymcp jaymcp added enhancement New feature or request [block] Header labels Feb 22, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@jaymcp jaymcp jaymcp approved these changes

At least 1 approving review is required to merge this pull request.

Assignees
No one assigned
Labels
[block] Header enhancement New feature or request
Projects
None yet
Milestone
v1.2.0
Development

Successfully merging this pull request may close these issues.
2 participants
@con322 @jaymcp