Feature 318: Slideshow homepage. Hero section #428
Conversation
|
@kelscahill please review this for code style and consistency. It is based on #318. |
|
Please don't merge. Will do some small fixes and push on Monday |
sergsadovyi
changed the title
.gitignore
Outdated
| @@ -8,3 +8,4 @@ yarn-error.log | |||
| .DS_Store | |||
| .package-lock.json | |||
| .yarn.lock | |||
| /views/patterns/01-molecules/components/carousel.blade.php | |||
There was a problem hiding this comment.
Why is this file ignored?
CHANGELOG.md
Outdated
| ##[3.0.33] | ||
| ### Feature | ||
| - Added 6 slide slider | ||
|
|
There was a problem hiding this comment.
Please don't edit the CHANGELOG.md file. These files will be edited after the merge and when I create the new release.
There was a problem hiding this comment.
@sergsadovyi and @eugeneweblab:
Before I review and merge these changes, I need you to resolve the conflicts in the merge. I have released a new version of the plugin SINCE you forked this, so please make sure those changes aren't over-written by this incoming merge.
When changes are incoming like this, please don't edit the CHANGELOG.md, style.css, kernl.version or changelog.json files. I will make changes in those files ONLY after the merge and when we are ready to create a new version.
Once the conflicts have been resolved in the merge, I will review the changes again.
Thanks!
style.css
Outdated
| @@ -2,7 +2,7 @@ | |||
| Theme Name: ALPS for WordPress | |||
| Theme URI: https://adventist.io/themes | |||
| Description: ALPS is the Adventist Living Pattern Library theme based off of the Sage WordPress theme. | |||
| Version: 3.0.32 | |||
| Version: 3.0.33 | |||
There was a problem hiding this comment.
Please don't edit these in the branch. This change will be made AFTER the merge.
sergsadovyi
changed the title
# Conflicts: # views/patterns/02-organisms/sections/page-header-hero.blade.php
There was a problem hiding this comment.
@sergsadovyi Have you check this from your end on the front? I'm getting an error when I try to view the page:
Switching to another theme, or older version, resolves the error.
I've tested on WP 5.3 with PHP 7.4. |
|
Try to add Hero Carousel slides from the scratch. Maybe a crash because of naming changes. |
|
@sergsadovyi I think I found the problem. The slide file was old and not updating correctly since it had been excluded from git. After I removed it, it worked fine. I think it looks great as your implemented it but after I played with it for a bit, I realized that the slides are building with the old centered layout, not the newer one here: Media Block - Inset. The version I asked you to use make it hard to see the text, whereas this one deals with that better. An example on other site: https://ministerial.adventist.org/ Can you please make that change? |
# Conflicts: # dist/scripts/customizer.js.map # dist/scripts/main.js.map # dist/styles/main.css.map
|
Updated the styles of the Carousel to match https://ministerial.adventist.org/. Maybe we should update original alps repo with Hero Carousel new styles? So the css for carousel will be not local but from alps cdn. (just a suggestion) |
|
@sergsadovyi It looks good, however, I'm seeing the images are scaling to fit the full width of the space, leaving a block on the right with out image. If the screen width is narrow enough, it look fine.
Regarding the CSS, yes, I would prefer that to come as part of the main CSS. Please make a pull request that includes this CSS for the ALPS Pattern Lab, as well as an updated pattern alternate that can use either this version of the slide or the other version of the slide. |
Can't reproduce a bug. @designerbrent Please provide an image, screen width and browser version. |
|
This is pending merge of the PR adventistchurch/alps#436 from ALPS Core. |
There was a problem hiding this comment.
@sergsadovyi Please remove the remaining CSS that got moved into the ALPS core project.
sergsadovyi
force-pushed
the
feat/318
branch
from
37e0edc to
72d9e8c
Compare
Closes #318