Website layout enhancement and code refactoring

[Ahmedsaed]

This PR contains bug fixes requested in issue #7 and other enhancements

Changes:

• Improved website layout
• Code refactoring
• Moved each component's styles into it's own file
• Solved merge conflicts

[netlify[bot]]

✅ Deploy Preview for prep-22-aug-prep-2-project ready!

Name	Link
🔨 Latest commit	6b5b253017e02bfa8c9edc1a5b39ade80f9c6ab3
🔍 Latest deploy log	https://app.netlify.com/sites/prep-22-aug-prep-2-project/deploys/62f7fedce7f8b5000844ba8e
😎 Deploy Preview	https://deploy-preview-48--prep-22-aug-prep-2-project.netlify.app/
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

---

To edit notification comments on pull requests, go to your Netlify site settings.

[farida52369]

Great work, Ahmed .. The refactor makes the code more reusable, and understandable (following the principles of OOP) But i have some observations:

• The Styling of get My location button is missing
• The layout of card-container, I think it was more UX before .. when opening the app as a user there's a lot going on in this row not to mention the styling for the required items need to re-fix.
• The background img doesn't cover all the stage

[Ahmedsaed]

Thanks fareeda,

• The Styling of get My location button is missing

I will fix the styling of the button (for some reason I forgot to add it while refactoring)

But I also have a comment, Why did you create another component just for one html line (A button). do you plan to expand it?

• The layout of card-container, I think it was more UX before .. when opening the app as a user there's a lot going on in this row not to mention the styling for the required items need to re-fix.

This PR contains improvements to the layout. The styling of the cards should be a new issue with a new PR

• The background img doesn't cover all the stage

I can't reproduce this issue as the background is working fine in the preview

[farida52369]

Thanks Ahmed,

according to

• Why did you create another component just for one html line (A button). do you plan to expand it?

I only solved the issue (Style the "Get My Location" button #42). It was another fellow who added the Component.

• This PR contains improvements to the layout.

I think the layout is less UX.

• The styling of the cards should be a new issue with a new PR.

I believe I've solved this issue before :)

• I can't reproduce this issue as the background is working fine in the preview

I suppose it's hard coded in App.css

padding-top: 20px;
padding-bottom: 20px;

[Ahmedsaed]

I only solved the issue (Style the "Get My Location" button #42). It was another fellow who added the Component.

Oh, I guess I will contact the fellow who created the component then.

I think the layout is less UX.

I think it would be more helpful if you provide suggestions

I believe I've solved this issue before :)

Yes, I actually used it in the refactored version I managed to reproduce the problem by zooming out, it seems that the root element is not taking the full height of the body

Even though this issue has nothing to do with the changes in the PR, but I will try to fix it

I suppose it's hard coded in App.css

padding-top: 20px;
padding-bottom: 20px;

Nope, the padding is part of the solution to the issue (we should use padding instead of margin)

[Ahmedsaed]

• The background img doesn't cover all the stage

Fixed it by adding

#root {
  height: 100vh;
}

To make the root element expand to the full height of body

, I will push the code in a moment

Edit: it seems that it's not fixed yet

[farida52369]

• I think it would be more helpful if you provide suggestions

we can add

<SearchComponent />
<ResultsComponent />
<Map />
<RequiredItems />

as Icons for weather state will be added to ResultsComponent So it will give the user more UX to focus on only one component per row.

• Fixed it by adding

  #root {
  height: 100vh;
  }

This solution seems to solve the problem but i still don't understand why when removing from App.css

padding-top: 20px;
padding-bottom: 20px; 

the map container looks this way :)

[Ahmedsaed]

we can add

<SearchComponent />
<ResultsComponent />
<Map />
<RequiredItems />

as Icons for weather state will be added to ResultsComponent So it will give the user more UX to focus on only one component per row.

I don't really know, these design decisions should be taken be the whole team

For me, I think one card in a row is a waste of space and it would require you to scroll down to view more info

This solution seems to solve the problem but i still don't understand why when removing from App.css

padding-top: 20px;
padding-bottom: 20px; 

the map container looks this way :)

This is a bug in the map, refresh the website or clear the cache and it should work fine

This bug might also happen while resizing the window, which is fine because users don't normally resize windows

[Ahmedsaed]

@farida52369

The background issue wasn't actually fixed for some scenarios but I managed to solve it by adding

background-attachment: fixed;

To to make the background fixed and prevent it from moving while scrolling

I will open a new PR with the fix

[farida52369]

@Ahmedsaed

Great Work, Ahmed