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

Feature/media aside updates #133

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

Feature/media aside updates #133

wants to merge 6 commits into from

Conversation

con322
Copy link
Contributor

@con322 con322 commented Jan 31, 2019

Proposed Changes

  • Alignment option so image/video can be on the left or right

@con322 con322 requested a review from jaymcp January 31, 2019 12:35
@jaymcp jaymcp added this to the v1.2.0 milestone Feb 4, 2019
jaymcp
jaymcp previously requested changes Feb 4, 2019
View reviewed changes
includes/blocks/media-aside/render.php Outdated Show resolved Hide resolved
src/scripts/blocks/media-aside/DisplayComponent.js Outdated Show resolved Hide resolved
src/scripts/blocks/media-aside/DisplayComponent.js Outdated Show resolved Hide resolved
src/scripts/blocks/media-aside/index.js Outdated Show resolved Hide resolved
@con322 con322 dismissed jaymcp’s stale review February 11, 2019 16:18

code changes

@con322 con322 requested review from jaymcp and removed request for jaymcp February 11, 2019 16:18
jaymcp
jaymcp reviewed Feb 12, 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, except this little oddity with the front-end styling. Not sure whether this is intentional or not.

Before:

After:

@con322
Copy link
Contributor Author

con322 commented Feb 12, 2019

@jonmcp fixed that little styling issue, looks like for some reason i'd added display: block; in not quite sure why reverted now anyways and works fine.

@con322 con322 requested a review from jaymcp February 12, 2019 15:47
jaymcp
jaymcp approved these changes Feb 13, 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] Media Aside 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] Media Aside 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