Skip to content
OverviewSolutions
3
Comments
11

Pavel B.

@JagholinEssen270 points

I’m a mysterious individual who has yet to fill out my bio. One thing’s for certain: I love writing front-end code!

Latest solutions

Latest comments

  • Pavel B.•270
    @Jagholin

    My browser says that it can't load stylesheet from google fonts. It appears that the URL in the <link rel='stylesheet'> is mistyped, which means the font isnt loading. Just FYI.

  • Pavel B.•270
    @Jagholin

    Great job! I only looked in DevTools, but havent found much to complain about.

    What can be improved:

    • It's just my personal preference, but in many cases you dont need display: flex; flex-direction: column, the default flow layout does many things just fine. It also does margin collapsing, which can be either good or bad(depends whether you are used to it or not).

    • width: 100% does nothing for display: block elements, and might even be harmful sometimes

    Marked as helpful
  • Pavel B.•270
    @Jagholin

    Styling is more or less fine, some things are misaligned. Phone number input field doesnt accept spaces, even though the placeholder text suggests that it should.

    "Next step" button for some reason has different styles on different pages. Sometimes it has cursor: pointer, sometimes doesnt.

    Next step is not a <button>, "go back" is neither a link nor a button. The second page has 0 interactive elements, according to accessibility tool in DevTools

    Dont use heading tags <h1>, <h2> etc. for styling, as they have special meaning for accessibility tools. Each page should have exactly one <h1> element. Skipping heading levels is unadvisable.

    Marked as helpful
  • Pavel B.•270
    @Jagholin

    Great job!

    Some suggestions for the future:

    • inlining CSS styles into HTML can be sometimes useful(from performance standpoint). However I suggest to forget that it's even possible and add links to .css files instead. In the future when you do larger projects, separating stuff into different files will help you with organization, and bundlers like Vite will take care of compiling your code into optimised, minified representations ready to deploy to the Web.
    • position: relative doesnt do anything here.
    • The task was to display the panel in the center of viewport, which is done only 50%. There are several ways of vertically and horizontally centering stuff, the one that I like is to add this
    body {
  height: 100vh; /*if the browser doesnt understand the next line*/
  height: 100dvh; 
  display: grid;
  place-content: center;
}

    accordingly, you dont need such large margins on top and bottom of your .main

    (in fact, these margins break layout for a range of heights).

    • instead of using width, I suggest using max-width. This simple change will make your layout more fluid and responsive to wider range of viewport widths.
    Marked as helpful
  • Pavel B.•270
    @Jagholin

    Container width as a % of viewport width looks and feels strange. It's better to just use some fixed values for min-width and/or max-width.

    The same goes for height, only in this case it's better not to assign any value to height, the default in most cases is appropriate. If you try to change the size of the viewport, you'll see the problem.

    (looking deeper, there are a lot of percentage-based heights and widths. I suggest removing them all unless necessary. Since you are using flexbox for many things, you should allow flex containers to decide appropriate widths/heights.)

    Marked as helpful
  • Pavel B.•270
    @Jagholin

    Ok, feedback:

    • indentation and consistent code formating is important, really important. Not only does it make reading your code easier, but also is an indicator of overall quality. If you are lazy, you can use automatic formatters like Prettier
    • Don't use <br> tags, unless absolutely necessary. HTML is for structuring your page, not for making some micro style corrections.
    • width: auto doesnt do anything, and width: 100% is in many cases either not needed, or outright harmful. Block elements automatically take up the entire available width space.
    • dont use position: absolute without a good reason. You dont even need it here, just place your text elements below the image. In your solution, this breaks layout for a range of viewport heights.

    So how to fix:

    • first, correct your structure. It should look something like this:
    .container
    .img
    .text
         .title
         .content
    • then, remake your styles to fit the new structure. Dont use magic numbers like top: 67%; left: 50%; Instead, let the layout flow naturally from top to bottom. Utilize CSS box model to tweak positioning of elements.
    Marked as helpful
View more comments