Skip to content
OverviewSolutions
6
Comments
7
Towkir Ahmed
@towkir

All comments

  • Towkir Ahmed•280
    @towkir

    Hi Melvin, Hope you're doing well and continuing making pixel perfect designs. The body background color is #FAFAFA and the cards are #FFFFFF. I believe you couldn't get the figma file, or you wouldn't miss that. Cheers.

  • Towkir Ahmed•280
    @towkir

    Loved the idea of content placeholder while api request is going on, great work. I have some feedbacks though:

    1. I'd recommend not clearing the search input after pressing enter or clicking the search icon, it helps when you're showing the "No definition found" message, keeps a track of the searched word.
    2. I searched for asdfa and observed the network panel, there are at least 4 api calls when words are not found, (404 responses).
    3. And then toggled to mobile view, typed nothing, just focused the search input and there were another 4 calls again.
    4. Changed back to desktop view and the same happened.
    5. The github repo is not visible, probably made private or something? you can check by opening the repo link from an incognito window.

    Cheers !!

    Marked as helpful
  • Towkir Ahmed•280
    @towkir

    Great work, congrats on completing the challenge. Here are some feedback from my end:

    1. The overall spacing between elements can be reviewed against the design, such as the items in the font selector dropdown and the list items in the word meanings, seems like there are a bit much spaces than the design.
    2. The font selector can be closed whenever user clicks outside the selector.
    3. The color of the horizontal lines can be reviewed, seems a bit too prominent.
    4. The bold fonts are not being rendered (such as for the word in h1 should have been bold or weight: 700), apparently you used variable fonts, need to check on that.
    5. It is recommended to have a generic font name declared as a fallback, such as:
      font-family: 'Inconsolata', monospace;
  font-family: 'Inter', sans-serif;
  font-family: 'Lora', serif;

    Cheers !!

  • Towkir Ahmed•280
    @towkir

    Great work and congrats on completing the challenge. Some feedback from my end:

    1. Add a throttle to the api call that fires on keypress of the input box, maybe 300 or 500ms, people who types really fast would be able to make less unnecessary calls. Or, maybe just fire the api call only on the search icon press or enter press. This way, the not found message won't show up in the middle of typing a word.
    2. Review the horizontal lines colors with the design, seems a bit prominent.
    3. Add more descriptive and smaller git commits (specially avoid adding the entire project in a single commit).
  • Towkir Ahmed•280
    @towkir

    Looks awesome. Loved the way you organized git commits. Some feedbacks though:

    1. There is a purple rectangle on focus of some clickable elements, such as the font selector down arrow and the audio player icon, probably you considered accessibility through keyboard but you can use focus-visible for that, this way, the border is only visible while navigating through keyboard and mouse click would keep the UI cleaner.
    2. When the dropdown is open, if you click outside the dropdown and anywhere else in the page, the dropdown can be closed, it feels better.
    3. The font names can be rendered in the corresponding font families, such as serif option rendered in the serif font and the mono option in monospace font.
    4. Apparently the font's that you tried to render is not being rendered because they're not linked properly, you kept them in the project directory but the project still doesn't know those fonts by their name, hence the fallback generic fonts are being rendered on my machine, probably it looks fine at your end because you might have them installed in your pc, you can have e look here
    5. Maybe don't clear the search box value after fetching the results.
    6. When the red border is visible in search box, if you click search, the purple border on the search icon collides with the red border.

    These are the things I thought could be improved on your next version. Cheers !!

    Marked as helpful
  • Towkir Ahmed•280
    @towkir

    I viewed it from a large monitor 32 inch, you can test wider viewports by zooming out your browser view. Few things:

    1. Remove the max-width property from your body tag.
    2. Use background-size: contain, that should fix the bg image not taking up the full width.
    3. Since you used min height of 100vh, it means you want to take the full height and not a scrollbar on the right unless the contents need scrolling. Try using these at the beginning at your css file:
    * {
  margin: 0;
  padding: 0;
}

    it's a css reset for the margin and paddings, or you can just use the 0 margin and padding on the body element, it removes the scrollbar from the right side of the screen, since the contents are not that long and doesn't require scrolling.

    Try to add more descriptive commit messages on git and try to avoid adding all changes in a single commit. It's a good practice.

    Left some comments on your git changes as well.

    Hope this helps.

    Loved your work. Cheers!!

    Marked as helpful
  • Towkir Ahmed•280
    @towkir

    Looks pretty cool, congratulations on completing the challenge. Would love to point out a few things that kept it from being a perfect one.

    • I feel like there is a bit of margin on the list items of the word meanings you can review it with the design.
    • The font selector makes a jump once you choose a font, let's try to keep that right aligned and always on the same place.
    • The selector is opened and if we click on any other place outside the dropdown, it should be hidden.
    • We should make more descriptive and smaller commits on git.

    Hope this would help, loved your efforts, it is really a nice project. Cheers

    Marked as helpful