@samuel-adu
Mirna Andrišić
@mandrisicAll comments
- @mandrisic
the solution looks great but you had a free figma template that explained all the sizes, padding, fonts etc., and it would have really helped you while solving this project.
some quick fixes that i also saw:
- i saw in some of your headlines you had a weird font and you could solve that problem with defining the font-weight
- when navigating in the mobile nav you didn't have the emphasis on which page the user is on (like you have on the desktop version, ::after)
- there are no smooth animations and transitions
these are just smaller things that could make your project better and if you get stuck in the project there is also a free video of this project
- @Dadv-a11y@mandrisic
this solution needs a lot of work. the mobile version was much better than the desktop. you should try and stick with the given design of the task (use the same font, try to get the right size of elements, paddings and margins etc). also, you should have an accordion that opens its description by also clicking the title (summary) of the question, not just by clicking the arrows. also it would be good if you would connect the nav elements features, pricing and contact with the correct sections, so that when user clicks on features in the navbar (the hamburger closes in the mobile version), it takes him to the features section on the webpage. it's easy to do and add it to the code.
Marked as helpful - @Dadv-a11y@mandrisic
you should try and make the style of your page be as close to the design preview images, as much as you can. also the transition of each slide changing looks a bit jarring because the arrows move as well, and only images and text should change. it would be better to just use a smoothe fade in transition. i also noticed that if i am on the slide 1 and click on the left arrow nothing happens, which means you didn't do the logic correctly. i looked at your js file and saw you used loops and conditions, but there's a much easier way to do all this. look at some of the other solutions of this task other people did and compare your design with theirs. you would then be able to see the mistakes more clearly.
- @craigwolfeWhat are you most proud of, and what would you do differently next time?
I would have to say I am most proud of figuring out the JavaScript. Understanding how to use forEach loops, swap images and use addEventListeners.
document.addEventListener('DOMContentLoaded', () => { const questions = document.querySelectorAll('.question'); questions.forEach((question, index) => { const answer = document.getElementById(`answer${index + 1}`); const icon = question.querySelector('.toggle-icon'); //hide all answers on load const toggleAnswer = () => { if (answer.style.display === 'none') { answer.style.display = 'block'; icon.src = 'https://github.com/myname/FAQ-Accordion/blob/main/assets/images/icon-minus.svg'; // Change to minus icon } else { answer.style.display = 'none'; icon.src = 'https://github.com/myname/FAQ-Accordion/blob/main/assets/images/icon-plus.svg'; // Change to plus icon } }; //swaps the icon and shows the answer when clicked question.addEventListener('click', toggleAnswer); //enter is pressed on the question the answer will toggle question.addEventListener('keydown', (event) => { if (event.key === 'Enter') { toggleAnswer(); } }); }); }); ### What challenges did you encounter, and how did you overcome them? One challenge I had was the path for swapping images in JavaScript will not display the images correctly in GitHub Pages. I had to retrieve the absolute path GitHub and use that in the JavaScript to get the images to render correctly. ### What specific areas of your project would you like help with? I think I figured out everything I needed to on my own with internet resources.@mandrisicactually you shouldn't have used javascript at all for this project, beacuse the challenge was to create an accordion with HTML nad CSS. it's best to use only HTML and CSS when you can, you just complicated everything for yourself
- @AlfiZainWhat are you most proud of, and what would you do differently next time?
Implementing SMACSS Methodology
What challenges did you encounter, and how did you overcome them?Implementing SMACSS Methodology
What specific areas of your project would you like help with?Everything
@mandrisicwhen i tried to open your hamburger menu an error showed up in the console so you should check that out. it all works how it should, the only tip that i can give you is to try and get the page as close to the design as you can. i find it helpful to zoom design pictures to be the same width as my screen, so it's easier to structure the layout.
- P@moustafaessamWhat are you most proud of, and what would you do differently next time?
I used vanilla framework for css styling.
What challenges did you encounter, and how did you overcome them?I encountered some challenges regarding styling using vanilla framework but reading the documentation helped me
What specific areas of your project would you like help with?Any comment will be highly appreciated!
@mandrisicit looks good but i have some tips:
- avoid using !important, here it really isn't hard to align content with flex or whatever else you choose to make it go to the 'left'
- use tags that already carry semantic meaning like <h1></h1>, instead of <div></div>
- @zahraabellu@mandrisic
at first look it seems really good, then i noticed you only designed the desktop version of the page, so it's not responsive. if you have time you should really design this page for more devices.
for that reason i can only give you advice on how to make the desktop version better:
- the avatar image should also be inside header so the border-bottom would also be below it
- each li element hover effect should go over the header border-bottom, which gives it the effect of being one border (which could be done with position: absolute and top values)
- button automatically puts different styles so you should've manually changed font-family, font-size, font-weight and so on
- you didn't design a lightbox gallery that was in design images
- if a customer chooses to go below zero for the amount of items and click the add to cart button it throws an error in the console - there are better ways of making the whole logic of the js much more easier and better
Marked as helpful - P@Helewud@mandrisic
it looks good but i just have few tips on how to improve this project:
- the overlay shouldn't just have a width od 32% it should go over other content except the open hambuger menu
- there is no need for 6 columns, 3 columns is the best option for your grid in this case, because i can see that the button 'Read more' and 02 section are not verticaly aligned -> and they would be if you put them in the same column (middle one)
Marked as helpful - @CLTJared@mandrisic
looks good, could use more padding here and there but overall very nicely done
- @ilham-boukWhat specific areas of your project would you like help with?
Any feedback are welcome
@mandrisicthe challenge was to make this task using only HTML and CSS, but in your code I saw you used JS as well. you should have used details and summary tags in html which would have made everything more accessible and easier for you to make. then you could also make the first summary item be open when the page loads so the user knows that they can open the other summaries as well.
- @zeeguWhat are you most proud of, and what would you do differently next time?
❤Things I've tried
- Added
aria-label="Rate 1 out of 5"for better screen-reader support - Considered web accessibility
- Scored 99% on Google PageSpeed Insights
- Used Mobile first approach for responsive design
Please let me know if there are any areas of improvement! Thanks :D
@mandrisicwhen I previewed your code the font Overpass didn't load and in the mobile version the containers of each rating number was I guess too big because they went to another row -> maybe it's even because it's not the correct font. all in all screenshot looks great, so good job
- Added
- @shadowbanksWhat specific areas of your project would you like help with?
I'm open to any other suggestions.
Thank you for your time :))
@mandrisicit looks good, keep up the good work
- P@kephaloskWhat are you most proud of, and what would you do differently next time?
Building the html structure completelly in plain JavaScript without a framework.
What challenges did you encounter, and how did you overcome them?Dynamically rendering html elements by creating reusable components.
What specific areas of your project would you like help with?n/a
@mandrisichello, I just threw a quick glance at your code and I have some things you could improve:
- you didn't use the font that was used in the design preview
- smallest mobile version 320px doesn't look good, it's not centered (probably because the container is too big)
- tablet version would look better if it also used the same design as the mobile version (everything in one column)
just practice and you'll se these things more clearly.
Marked as helpful - @AlexDDevv@mandrisic
i couldn't see your code or your site live. but from what i can see through the sceenshot it looks okay, just a little small
- @danielashjariWhat are you most proud of, and what would you do differently next time?
- Use frameworks
- write cleaner lines of code
- Have aria attributes and accessibility in mind from beginning
- pop-ups => use js and event listeners
- aria attributes
- pictures (tags, styles)
@mandrisiceverything works fine, but you should focus more on structuring and designing projects as close to the design files
tip: if you want your component to be centered on the page make .attribution container have a position: absolute, so it doesn't affect the rest of the page alignment
Marked as helpful - @hkaur108@mandrisic
there's a couple of things you didn't copy from the given design:
- quotation mark image is not visible (if you're having trouble understanding how to make the image go 'behind' the content and still not affect the rest of the content: read more about position: absolute and z-index)
- alignment of text - everything should be on the left, and have less padding
- cards should end on the same horizontal line
- you shouldn't have a header inside a header
- the testimonials should be centered on the page
i hope this helps little and doesn't sound too mean
Marked as helpful - @dharul619@mandrisic
the desktop version looks good, but the mobile version could do some work. the 'boxes' were too big for the screen. it's better to start from the mobile version and then do the desktop version. in the code i saw that you used box-container to group the box-2 and box-3 to go in the middle, but you could have just used grid and would've saved a lot of time.
Marked as helpful - @je0va@mandrisic
for the desktop version the only thing I could see, that you didn't horizontaly center the prices (align-items: center), but thats a minor thing. mobile version could be better, prices could use more padding, container cuts on the top, there should be more space between the product container and the background.keep coding and you'll see these mistakes more clearly.
