I'm having some trouble displaying the modal and hiding the main content when i press the subscribe button
Mynor Zuñiga
@mynorzsAll comments
- @hakeemgardnerSubmitted over 1 year ago@mynorzsPosted over 1 year ago
Hello!
I see that you are having the problem I had hiding the maing content. I beleive there are different ways to go over this, but I will share how I found out and how I implemented it in my solution.
The main content is not hiding because your subscribe button is submitting the form, and submitting the form reloads the whole thing, so that default action needs to be prevented. In the click event listener, in the function you can pass a parameter, let's say "e" or "event", and inside the function you can take that parameter and apply .preventDefault(), so it would be something like this:
yourbutton.addEventListener('click', (event) => { even.preventDefault(); })
In your case, try: function handleModal(event) { event.preventDefault(); modal.classList.add("visible"); main_content.classList.add("hidden"); }
I hope that works for you. I'm fairly new myselft, so I'm sorry if I cannot explain myself very well here.
1 - @rafsznSubmitted over 1 year ago
PLS RATE MY WORK:)
@mynorzsPosted over 1 year agoHey, I liked your solution.
I will take the liberty to give you some visual feedback, since my main career is UX/UI Design and I have long years of experience giving visual review to developers:
You could go for more fidelity with margins and paddings to match the design as much as possible. On mobile, not caring about margins can lead to a confusing reading, not knowing where each section ends. It gets a weird reading to have paddings and margins too different, you could try defining a consisten margin, padding and gap that is easily repeated.
On mobile, the Read More button is too small, not only the size of the button, but also the font size. Right now you have it at 31px height, you could try not going below 44px, it will match the design and will represent a well implemented usability practice.
You could try delivering a responsive experience instead of an adaptive one. Right now it just snaps between mobile and desktop, but if you put a tablet width in the browser, it displays the mobile design on a reduced manner. I know that specifications do not require all devices, but by making it responsive you cover your back a bit more and the mobile design could expand instead of remaining in one fixed width.
I hope my tips can help you, and I wish you the best.
0 - @afrentadorSubmitted over 1 year ago
Es agradable hacer estas prácticas que te demuestran que si se puede avanzar en este camino tan bonito e intrincado, aún espero culminarlo, me falto una característica, pero estoy tan entusiasmado que prefiero subirlo de una vez espero les guste...
@mynorzsPosted over 1 year agoHola,
Me ha gustado tu solución. A pesar de que es una implementación simple, considero que es una muestra de cómo puedes llegar a trabajar proyectos más avanzados, ya que la fidelidad visual de tu solución, comparada con el diseño, es total.
Felicidades y comparto el entusiasmo, yo me encuentro en una etapa de aprendizaje, iniciando el largo camino a convertirme en full stack o al menos font end developer. Espero poder ver que sigas subiendo soluciones.
Marked as helpful0 - @mahassanSubmitted over 1 year ago
This project took me long then normal, much due to my office work but also because of getting to understand semantics, and I hope I did work out better.
- Did I made semantic more clear than my previous commits?
- What other feedback you can give me if I revisit this challenge?
@mynorzsPosted over 1 year agoHi, I'm still learning code, but I can give you some feedback regarding the visual accuracy of your implementation. I'm a UX/UI designer and I'm used to give visual reviews of implemented products, so I hope I give you some useful advice that can polish your deliverables.
- When you switch to the desktop version, the card is not showing the drop shadow that the design specifies, so the white halve of the card gets lost with the background.
- On the violet part of the card, check for the accurate distribution that the design presents. The design specifies an equal or even distribution vertically, but yours is stacked to the top, without respecting gaps and margins.
- In the summary section, each list item's background is to opaque, you could try making it more transparent. Also, the elements inside each item are evenly distributed, while the design presents and spaced between distribution, between two groups: icon+title ----- progress numbers.
- Inside the summary items, the progress shows two different styles in the original design, the completed amount has to show with a heavier font weight.
Those are some points you could review to polish your game for visual reviews.
I hope this was of some help.
1