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

WIP - Let The Loop be an amp-live-list #49

Closed
wants to merge 28 commits into from
Closed

WIP - Let The Loop be an amp-live-list #49

wants to merge 28 commits into from

Conversation

westonruter
Copy link
Contributor

No description provided.

@kopepasah
Copy link
Contributor

@westonruter it is likely this will need to be reworked following the work in #43 (branch add/34).

@westonruter
Copy link
Contributor Author

@kopepasah sorry I missed that 😦

@westonruter westonruter mentioned this pull request Jan 29, 2018
Merged
@westonruter
Copy link
Contributor Author

@kopepasah I was going to go through and make the changes but I don't want to mess with the way you're doing markup. Would you please make the necessary adjustments so that we can make use of amp-live-list for The Loop, either in this PR or another?

I'm not sure about the wrapper element around the article. For the live list to work the elements in the loop need to have an id attribute and data-sort-time attribute: https://www.ampproject.org/docs/reference/components/amp-live-list#attributes-on-items-reference-point-children

I think that adding amp-live-list is a compelling demonstration of this AMP component.

@ThierryA
Copy link
Contributor

ThierryA commented Feb 1, 2018

@westonruter can this be closed?

@westonruter
Copy link
Contributor Author

@ThierryA I'm not sure as I think @kopepasah or @delawski were going to make changes to the loop to allow for it to be an amp-live-list. I saw Loop do it for more stories partial, but I haven't seen it yet for the main loop.

@delawski
Copy link
Contributor

delawski commented Feb 1, 2018

You're right, @westonruter . @kopepasah or myself are going to change the way the articles in the amp-live-list are being used.

@delawski
Copy link
Contributor

delawski commented Feb 2, 2018

@westonruter Here's a PR that simplifies the markup: #67

I haven't been working on this PR since it's out of date and I wasn't sure if it's going to be used at all.

ThierryA and others added 16 commits February 2, 2018 18:17
@ThierryA
Apply DRY principals
600b7df
@westonruter
Simplify home feature loop logic
6a496cd
@westonruter
Eliminate need for wrapper div by introducing additional post classes
5acc621
@kopepasah
Rework home template to use amp-live-list.
5b0cef1
@kopepasah
Merge branch 'develop' into add/the-loop-amp-live-list
4b83da0 
# Conflicts:
#	archive.php
#	inc/template-tags.php
#	index.php
#	search.php
@kopepasah
Use amp-live-list on single articles.
d7e0b2a
@kopepasah
Fix button styles for amp-live-list.
fe57314
@westonruter
Add missing the_post() call to advance the loop
f7ff925
@kopepasah
Rework slim template to be amp-live-list compatible.
7e566e1
@kopepasah
Use amp-live-list on category template.
38e6d4a
@kopepasah
No need to wrap archive header in have_posts() conditional.
e3483ea
@kopepasah
Use a template for posts live lists.
5621b06
@kopepasah
Add live lists to archive template.
c149083
@kopepasah
Add posts live list to search template.
661f35c
@kopepasah
Use live list on page template.
5884840
@kopepasah
Build assets.
329cc58
@kopepasah
Copy link
Contributor

@westonruter I pushed several updates which use amp-live-list on most of the content. The only current exclusion is home.php template (see comment). I think this implementation is much more practical than the one happening in #67.

@delawski
Merge branch 'develop' into add/simplify-entry-markup
6333fd4 
* develop:
  Fix comment styling
  Issue 38: add escaping
  Enqueue font instead of using @font-face
  Compile assets.
  Issue 38: Make the header sticky on mobile and desktop.
  Compile assets.
  Issue 38: Add padding to the header nav menu.
  Issue 38: Fix desktop header search form alignment.
  Issue 38: Fix desktop main menu layout.
  Issue 38: Revamp site header and menu on mobile.
  Build assets.
  28 Add widget class for recent comments.
  Build assets.
  28 Add specificity for recent comments widget.
  28 Add styles for recent comments widget.
  38 Set state for mobile menu.
  38 Add toggle state of… toggle.

# Conflicts:
#	assets/dist/css/main.css
#	assets/src/css/base/_structure.scss
@delawski
Update static markup.
fa32f9b
@delawski
Merge branch 'develop' into add/simplify-entry-markup
3c20b01 
* develop:
  Issue 38: Fix site header width on large desktop.

# Conflicts:
#	assets/dist/css/main.css
@kopepasah kopepasah changed the title Let The Loop be an amp-live-list WIP - Let The Loop be an amp-live-list Feb 6, 2018
@kopepasah
Merge branch 'add/simplify-entry-markup' into add/the-loop-amp-live-list
a2631c7 
# Conflicts:
#	archive.php
#	assets/dist/css/main.css
#	category.php
#	functions.php
#	search.php
@kopepasah
Merge branch 'add/simplify-entry-markup-2' into add/the-loop-amp-live…
c35a129 
…-list

# Conflicts:
#	assets/src/css/main.scss
#	home.php
@kopepasah
Build assets.
534c984
@kopepasah kopepasah mentioned this pull request Feb 6, 2018
Closed
kopepasah
kopepasah suggested changes Feb 6, 2018
View reviewed changes
Copy link
Contributor

@kopepasah kopepasah left a comment

Choose a reason for hiding this comment

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

There have been quite a few updates to this pull request and a bit more work is needed (to be completed by myself).

@kopepasah kopepasah closed this Feb 6, 2018
@kopepasah kopepasah mentioned this pull request Feb 6, 2018
Merged
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@kopepasah kopepasah kopepasah requested changes

@delawski delawski Awaiting requested review from delawski

Assignees

@delawski delawski

@kopepasah kopepasah

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@westonruter @kopepasah @ThierryA @delawski