Skip to content

Periklis K #152

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

Open
wants to merge 17 commits into
base: main
Choose a base branch
from
Open

Periklis K #152

wants to merge 17 commits into from

Conversation

PerikK
Copy link

@PerikK PerikK commented Mar 14, 2024

No description provided.


<div class="user">
<div class="profile-pic">
<img src="./svgFiles/Hello Ctulhu! black letters.jpg" alt="" width="45px">
Copy link

Choose a reason for hiding this comment

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

I would avoid having spaces and special characters (!) in file names, they can cause unexpected issues on certain systems. Better to go with underscores and dashes just to be safe

</g>
</svg>

<!-- <div class="icon-spacer"></div> -->
Copy link

Choose a reason for hiding this comment

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

Try not to commit dead code like this. We can always go back to previous commits, that's what git is for, so don't worry about keeping things you might need in future

Comment on lines +1264 to +1266



Copy link

Choose a reason for hiding this comment

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

There's a lot of chaotic white space in this file...

display: grid;
grid-template:
'header main news' /
25% 1fr 30%
Copy link

Choose a reason for hiding this comment

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

It can be difficult to work with %'s for layouts, it's better to define fixed pixel widths for side bars and change them with media queries at certain break points

Copy link
Author

Choose a reason for hiding this comment

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

Oh god! And I thought that %'s will be better for responsive design since analogy stays constant no matter what the actual size is. Duly noted as are the rest of your remarks.

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.

2 participants