Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

new design #66 #70

Merged
merged 4 commits into from
Apr 15, 2019
Merged

new design #66 #70

merged 4 commits into from
Apr 15, 2019

Conversation

rocktimsaikia
Copy link
Contributor

@rocktimsaikia rocktimsaikia commented Apr 13, 2019

Hi in this pull request I made few changes in the overall home page design .All the designs specs is based on the issue #66 that I opened few days ago.

So again these are the things i worked on :

  • Got rid of the heavy background image that was not optimized instead of went for a more simple flat design with a simple vector png which made the loading time for the page more faster.
  • Used some simple basic css design principles to make the web page look more professional and standard( like neon gradients ,flat design color palettes ,standard google fonts).
  • Made the footer stick to bottom and inside the footer added a contribution button linking to this repo.

here is the final result :
brewer

Please let me know what you guys think of these chnages .

@rocktimsaikia rocktimsaikia changed the title new design #66 new design [#66] Apr 13, 2019
@rocktimsaikia rocktimsaikia changed the title new design [#66] new design #66 Apr 13, 2019
Copy link
Collaborator

@cdrani cdrani left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@rocktimsaikia: I really like this new design. The linear gradients are really striking. I have included some changes, with most of them just being cleanup.

A few changes I haven't included in the comments below as they could be their own issues:

  1. back to search button text color is not easily readable against the gradient:
    Screen Shot 2019-04-14 at 00 36 23.
    It should match the style of the Check them out buttons.
  2. Hovering over the buttons shows the unrounded form of the rectangular button.
    Screen Shot 2019-04-14 at 00 35 23

I am really impressed otherwise. I advise just making the changes requested, but if you feel up to it, then make the two changes above as well. If not, then a new issue could be created from them and offered to someone else.

Thanks for contributing! I am looking forward to seeing this merged in.

Can you also offer your opinion on this PR @JasonFritsche?

src/App.css Outdated Show resolved Hide resolved
src/App.css Outdated Show resolved Hide resolved
src/App.css Outdated Show resolved Hide resolved
src/components/BrewerySearch.js Show resolved Hide resolved
@rocktimsaikia
Copy link
Contributor Author

@cdrani Thanks for the review i will make your requested changes asap 😊

@rocktimsaikia
Copy link
Contributor Author

rocktimsaikia commented Apr 14, 2019

@cdrani hi I made the requested changes that you mentioned. So again here is the overview of the new changes i made :

  • Removed the unused old google fonts that were taking time to load.
  • Moved the body selector css part to index.css file .
  • Used classes over ids [Except few specific ids as they are needed for the logos] and added newline between every css selector
  • Replaced commonjs require with es6 modules.
  • Fixed the gradient background issue that was making the go back button hard to read (I just changed the color from soft white to plain standard white on that button specifically for better readability ).
  • Removed the unnecessary messy comments.

A few new changes I made :

  • Changed the background color of search input field from gradient to plain white.Which looks more clean now.
  • Added gradient background to cards and made the card buttons block level which looks much better now.

Here is the changed homepage with the white colored input field :
new site

Search result page with new gradient cards :
bak

@JasonFritsche JasonFritsche merged commit f8e2731 into JasonFritsche:master Apr 15, 2019
@JasonFritsche
Copy link
Owner

@rocktimsaikia Thanks for your contributions to the project. This is an awesome addition.

@rocktimsaikia
Copy link
Contributor Author

I am glad i could use this holiday season working on open source rather than my college stuff :)

@JasonFritsche
Copy link
Owner

Feel free to make more PR's. If you see a bug or feature that is not in the current set of issues then feel free to open a new issue. Thanks!

This was referenced Apr 15, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants