search

dhood / dhood.github.io

Other
0 stars 2 forks source link

Updated font-awesome from 4.5.0 to 6.5.1 #6

Closed CharlzGaldo closed 9 months ago

CharlzGaldo commented 10 months ago

All added updates in this branch:

Things that still need to be done:

dhood commented 9 months ago

Yay, you updated the font so we can use the new icons! That's awesome, I didn't know how hard that would be, but it seems to all be working well... the new icons look great! :raised_hands:

Pointing out the problems you've already noticed was really helpful, thanks, because it saves me time reviewing. Now I can just skip straight to the advice!

Change the color of the font-awesome symbols on the site from dark grey to green (Like the original)

I switched back to the master branch to check what was giving them colour in the first place, using "ctrl-shift-c" + inspector. It was coming from the "icon" class, the css file sets them green if they have the "icon" class tag. Do you remember why you changed the "icon" classes to be "fas"? It might not be necessary to do that, or if it is necessary, perhaps you can use BOTH the fas and icon classes?

(oops, the gif doens't show my mouse. I'm clicking on the button to the left of 'inspector' and then choosing the object on the website that I want to see the css properties of - you have to just imagine :laughing: )

icon-colour

Fix navigation button fonts at the top ("STAY IN TOUCH" doesn't fit in the black box)

Same sort of thing - the "font awesome brand" font is being applied to the fonts now via the "fab" tag where it used to be something from the website style. Perhaps reverting the "fab" usage fixes it? :thinking: font-fab

Add a gap between the new symbols and text because it looks too close and cluttered.

I like it close! otherwise you're not sure what text relates to which icon...

Although, the text sometimes gets split away from the icon in mobile version, depending on the window width.. perhaps you can either make the instagram logo always stick with the instagram text somehow, or put them across two lines (two on each line) in the mobile version? Screenshot from 2023-12-29 09-29-12

CharlzGaldo commented 9 months ago

All problems Fixed,

CharlzGaldo commented 9 months ago

All of the problems were created because I was using "fas" instead of "icon". 😅 I was using "fas" because it didn't work when I used "icon" for some reason, but now it works and I don't know how or why😆, sorry for the delay.

Note: I kept "fab" for the logo because I don't know how to make it work without it.

CharlzGaldo commented 9 months ago

Additional Change:

dhood commented 9 months ago

this looks EXCELLENT! :raised_hands:

Only small things I noticed is that: 1/ the instagram logo gets outside the circle on small screens - do you know what that's about?

image

2/ Could you give them four unique colours? something in the theme of the background image colour palette?

3/ Also if you find a way, the instagram text seems to still get separated from the instagram logo for some screen sizes (as above). Do you reckon you could find a way to fix it? The easiest might be to just make sure they end up below the image, somehow, because then they can stay all on the one line. (Or split them across two lines, whatever you prefer!)

CharlzGaldo commented 9 months ago

Added the changes. But this is what it looks like with the text underneath the logos. I don't really know how to center it, but I'd love to learn if you do. image

Also, sorry for the minor mistakes, I'll try to keep it cleaner in the next few updates.

dhood commented 9 months ago

that looks nice! and don't worry, iteration like this is normal.

To get Instagram text to be centered underneath the logo, you want to find a way to GROUP those two things together, and then within that group, to apply some centering. (and then do this for the other four).

I think they're already grouped together with a 'span', where it say <span class="fa-stack". So you can add style="text-align: center" to the span grouping, and it should center them. But you might also have to specify the width of the span e.g. width: 80px

Want to give it a go?

dhood commented 9 months ago

I still not sure how to fix the problem in the first picture I posted where the 4 icons get split for medium screen sizes... I will have a think about it!

CharlzGaldo commented 9 months ago

Thanks! I finished centering the logo and text, and now have a better understanding of how to center things. I also added a height: 10% in the style so the logos would stay on the same line. It works but for some reason, there is just one size that doesn't work. "shown below". It seems to fix if I get rid of the width: but then the text isn't centered. image

I also don't understand which image you are referring to in your last comment. 😅

CharlzGaldo commented 9 months ago

the Instagram logo seems to only be a problem with that specific screen space/resolution. All the other ones work

dhood commented 9 months ago

this looks great - thanks so much for the fix!! we can maybe fix the split in another pull request (taht's what I was talking about, too!) - for now, this deserves to be deployed to the live website!! :raised_hands: