NEAR-Edu / near-certification-tools

2 stars 2 forks source link

Feature/layout #7

Closed ozanisgor closed 2 years ago

ozanisgor commented 2 years ago

BEFORE:

10 02-22 before

AFTER:

image

ryancwalsh commented 2 years ago

@ozanisgor @norrec99 @rashaabdulrazzak One new guess about the command line error you experienced related to pushing: I wonder if it was something to do with the branch name capitalization. E.g. Feature/layout vs feature/layout. Did you ever accidentally type it with uppercase F? I don't know.

ryancwalsh commented 2 years ago

@ozanisgor This branch PR looks pretty good except that the header now is confusing since the Jobs link feels so out of place.

image

I wonder if we should just delete the Jobs link.

And maybe play around with the footer and see whether you think this would look better:

Another concern is that we don't want to have any dependencies in any project that lack licenses. I'm not sure about https://fortawesome.com/start It looks like we'd need to pay for it. Aren't there similar options that are free and offer licenses stating that anyone can use them (even for-profit organizations)?

ryancwalsh commented 2 years ago

@ozanisgor Maybe the free version of https://fontawesome.com/plans has the social icons you want? You can check. fortawesome is a different spelling from fontawesome and is a different project. I'd never heard of fortawesome until I saw you use it. I think FontAwesome is popular.

ozanisgor commented 2 years ago

@ozanisgor Maybe the free version of https://fontawesome.com/plans has the social icons you want? You can check. fortawesome is a different spelling from fontawesome and is a different project. I'd never heard of fortawesome until I saw you use it. I think FontAwesome is popular.

@ryancwalsh Actually, I used the Font Awesome free version. I followed the steps in this website https://fontawesome.com/docs/web/use-with/react/

yarn add @fortawesome/fontawesome-svg-core
yarn add @fortawesome/free-solid-svg-icons
yarn add @fortawesome/free-regular-svg-icons
yarn add @fortawesome/react-fontawesome@latest

I think they are using fortawesome naming for installation. I thought they are using this naming because it is funny.

Maybe I am doing and seeing it wrong. I thought Font Awesome's free version works for us but I am not sure of course. If you suggest not to use Font Awesome, I may continue with something else as you suggest before.

ryancwalsh commented 2 years ago

@ozanisgor Interesting! I definitely assumed the different spelling was from a different library. But you're right. Ok, well, if it's free and if the license says that we're allowed to use it, and if it has the social icons we want, great!

ryancwalsh commented 2 years ago

@ozanisgor I just pulled this branch to have a fresh copy locally but still don't see any changes that I mentioned at https://github.com/NEAR-Edu/near-certification-tools/pull/7#issuecomment-1039434540 about the Jobs link or the footer. Have you played around with those yet to see what would look best? Screenshots (and your opinions) would be helpful. Thanks.

ryancwalsh commented 2 years ago

@ozanisgor Also, can you please delete the h1 element from the page at http://localhost:3000/certificate/eb9c618b235649728883ce13d9ce1247 ?

image

We will come up with some better text later, in a future version of this page, once we're efficiently reading on-chain data.

ozanisgor commented 2 years ago

@ozanisgor I just pulled this branch to have a fresh copy locally but still don't see any changes that I mentioned at #7 (comment) about the Jobs link or the footer. Have you played around with those yet to see what would look best? Screenshots (and your opinions) would be helpful. Thanks.

@ryancwalsh Actually, yesterday changes were almost ready but I wanted your approval for the Font Awesome and push them all together because every time when I push and rebase I got some git errors and I need to re-apply my changes. Maybe I should record a loom video to show what I mean by this. For my next assignment I will try to record my every git step.

Btw I can still remove the jobs if it is not looking good.

Created new component for the icons because EsLint's max line rule.

  1. Consolidate into one row
  2. Add a full-width horizontal line above
  3. Make the entire footer background color light gray

after-16 02 22_v1


after-16 02 22_v2


image

ryancwalsh commented 2 years ago

You're making progress.

  1. I think it would look better if the entire footer were left-aligned. And the contents would be: NEAR Univ logo, then social icons, then Jobs link.

  2. Are you sure you've pulled develop locally to make sure develop is up to date on your local machine and then check out your branch again and do git rebase develop? It sort of looks like this PR is showing lots of changes to web-app/pages/index.tsx that it shouldn't be showing since they already exist in the develop branch. https://github.com/NEAR-Edu/near-certification-tools/blob/develop/web-app/pages/index.tsx

  3. Can you fix the weird lack of padding that is happening on narrow screens such as for here?

image
ryancwalsh commented 2 years ago

@ozanisgor At https://near-certification-tools-tpq1.onrender.com/account/hatchet.testnet it would also be good to decrease the size of the heading and remove the spacing from above and below it:

image

And at https://near-certification-tools-tpq1.onrender.com/certificate/eb9c618b235649728883ce13d9ce1247, let's decrease the size of the certificate since it's too big and requires the visitor to scroll to much:

image

Maybe max-height: 500px;? And make sure it's responsive.