Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions comments.MD
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Comments / things that stand out

- Grid is always 12 columns
- For the first task, I just changed classNames, because its the easiest solution.
- For the second task, I moved the markup of the icons down, also removed some CSS that added element-spacing to the bottom of the item. I realize it's already handled by a `:last-child` selector in the _critical.css but I think for maintainability and readability sake it's better to also remove it from the component specific styling. I think if a new element was introduced underneath, it would need to be re-evaluated anyways with regards to how much whitespace there should be between items. I also made it so that the icons are aligned at the bottom with each other to make the visuals seem not as chaotic.
- Why is 1 of the USP items a link while the others arent? for now I'll just assume it was a mistake and remove the link element. The only reason I'm doing so is because I want to add flex and I would have to add flex to 2 different items.
- For the reviews section with regards to headings; I think there might need to be some change anyway; none of the texts on their own are a very good indicator of the section in the page structure. Similarly, the USP's could do with a (visually hidden) heading 2, and potentially the actual USP items could get a heading 3 (their title)
- Also no completely sure about the headings in the slider hero.
- For the touch target size, I've upped the size on mobile. The button is still touching the edge of the input field.I don't know if this is intentional design wise, so I left it (I checked the initial styling). Also, the + is falling out of the button, so I added some markup + styling to fix this.
- While making the new section I tried to use as much existing styles as possible, and for the design implementation I had to eyeball some values, one of the things I eyeballed is the margin between image and content on mobile. I did not do it the same way as it's done in the `two-column-info-block` slice, which uses margin as a %. Instead I used a static value, as I am not sure what your guys' philosophy is on this.
- For the responsiveness of this section, I tried to use the same way of thinking as was used in the `two-column-info-block`. To be honest the sections are now very similar but I would imagine ina. real life situation there wouldbe some back and forth regarding design.
- For the collapsable footer, I have some improvements /comments if I had a little more time. Firstly, an animation on the closing/opening would be nice. Second, I have the feeling I might have to dive a little deeper in the markup and stying as I saw some `collapsible` class already pop up somewhere.
- I implemented the scroll functionality with JS, though we could also implement an anchor to scroll to and add the scrolling-behaviour to the body.
- For making sure that content is always readable, I added a padding-top with currently a set in stone value, which is not based on any vars. I think in some cases it's worth considering making a calculation out of this (for example, padding-top and bottom of nav item + font-size).
122 changes: 77 additions & 45 deletions src/index.html

Large diffs are not rendered by default.

23 changes: 23 additions & 0 deletions src/js/footer.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,13 @@
class CollapsibleFooter extends Collapsible {
constructor() {
super();

this.listHeadings = this.querySelectorAll('.collapsable-list button');

this.listHeadings.forEach((list) => {
list.addEventListener('click', (e) => this.toggleList(e));
});

if (!this.options.breakpoint) {
this.options.breakpoint = parseInt(768);
}
Expand All @@ -19,17 +26,33 @@ class CollapsibleFooter extends Collapsible {
this.displayDetails();
});
}

toggleList(event) {
const target = event.target.closest('.collapsable-list');
const isOpen = target.getAttribute('data-open');
if(isOpen === 'true') {
this.close(target);
} else {
this.open(target);
}
}

open(group) {
if (window.innerWidth < this.options.breakpoint) {
super.open(group);
}
group.setAttribute('data-open', 'true');
}

close(group) {
if (window.innerWidth < this.options.breakpoint) {
super.close(group);
}
group.setAttribute('data-open', 'false');
}

openAll(){

}

displayDetails() {
Expand Down
13 changes: 12 additions & 1 deletion src/js/swiper-slider.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,12 @@
constructor() {
super();
this.swiper = this.querySelector('[data-swiper]');

this.scrollIndicators = this.querySelectorAll('.swiper-scroll-indicators') || [];

this.scrollIndicators.forEach((indicator) => {
indicator.addEventListener('click', () => this.scrollPastSlider());
});

// Stop when the swiper is not found
if(!this.swiper) {
return;
Expand Down Expand Up @@ -135,6 +140,12 @@

}

scrollPastSlider() {
const boundingRect = this.swiper.getBoundingClientRect();
const sliderEnd = boundingRect.bottom - boundingRect.top;
window.scrollTo({ top: sliderEnd, behavior: 'smooth' });
}

/**
* Handles the theme editor block change/edit event
* @param {Object} event
Expand Down
6 changes: 5 additions & 1 deletion src/scss/component/component-usp.scss
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,10 @@
padding-inline: var(--site-side-spacing-xxl);
}
}
&__wrapper {
display: flex;
flex-direction: column;
}
&__title {
margin-bottom: 0;
@include mqmax($bp-md) {
Expand All @@ -29,7 +33,7 @@
&__icon {
display: inline-flex;
padding: rem(10);
margin: 0 auto calc(var(--element-spacing) * 3) auto;
margin: auto auto 0 auto;
transform: scale(1);
transition: transform var(--duration-default) ease-in-out;
@include mqmin($bp-xl) {
Expand Down
25 changes: 23 additions & 2 deletions src/scss/section/section-footer.scss
Original file line number Diff line number Diff line change
Expand Up @@ -138,10 +138,21 @@
width: rem(20);
height: rem(20);
padding: 0;
@include mqmax($bp-md) {
width: rem(24);
height: rem(24);
}
.icon {
width: rem(32);
height: rem(32);
display: flex;
justify-content: center;
align-items: center;
margin: 0;
width: rem(16);
height: rem(16);
@include mqmax($bp-md) {
width: rem(20);
height: rem(20);
}
}
}
.field__input {
Expand Down Expand Up @@ -214,6 +225,14 @@
}
}
}
&[data-open="false"] {
@include mqmax($bp-md){
.level-2 {
max-height: 0px;
overflow: hidden;
}
}
}
}
&__link {
display: block;
Expand All @@ -229,6 +248,8 @@
font-size: rem(26);
line-height: rem(31);
letter-spacing: rem(0.5);
background: transparent;
border: none;
@include mqmin($bp-md) {
font-size: rem(32);
line-height: rem(42);
Expand Down
6 changes: 4 additions & 2 deletions src/scss/section/section-hero-slider.scss
Original file line number Diff line number Diff line change
Expand Up @@ -25,9 +25,9 @@
&__content-wrapper {
box-sizing: border-box;
// padding left/right given to avoid overlap with the "swiper navigation arrows"
padding-block: 0 rem(100);
padding-block: rem(80) rem(100);
@include mqmin($bp-md) {
padding-block: 0 rem(102);
padding-block: rem(80) rem(102);
max-width: var(--site-center-narrow);
}
&.has-navigation {
Expand Down Expand Up @@ -133,6 +133,8 @@
left: 50%;
transform: translateX(-50%);
bottom: rem(32);
background: transparent;
border: none;
@include mqmin($bp-lg) {
bottom: rem(50);
}
Expand Down
12 changes: 12 additions & 0 deletions src/scss/section/section-text-image-block.scss
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
@use '../mixins'as *;

.section-text-image-block {
.text-image-block {
&__image {
margin-bottom: rem(24);
}
&__description {
margin-top: var(--spacing);
}
}
}