Skip to content
OverviewSolutions
9
Comments
15
Fayaz
@fayax555

All comments

  • Fayaz•380
    @fayax555

    Take a look at CSS modules. It creates local CSS instead of global. With this approach, conventions like BEM become unnecessary.

    To use it with React, change the file name to someComponent.module.css. and import it in your files like `import styles from './someComponent.module.css'.

    And use it in your classname. <section className={styles.filterbox}>.

    If you have any more questions reply to this comment.

    Marked as helpful
  • Fayaz•380
    @fayax555

    You could prefix with an S instead of Styled. So StyledNavbar will become SNavbar.

  • Fayaz•380
    @fayax555

    Filter logic is not correct. When you click frontend tag, you should see all jobs that have frontend tag.

    if (filterList.some(tag => filters.includes(tag))) return null; I think if you change some to every, it will work correctly.

    Marked as helpful
  • Fayaz•380
    @fayax555

    According to challenge requirements, users should be able to search for any domain.

    You can do this by changing ipAddress to domain in fetch(`https://geo.ipify.org/api/v2/country,city?apiKey=at_qoJBZrBxMVeTgrJkk7P3vZIbtmb0U&ipAddress=${address}`).

    Other than that, this looks great to me.

    Marked as helpful
  • Fayaz•380
    @fayax555

    Really nice solution.

    One thing I would add is, when you switch to 'Yearly Billing', it should not change from '/monthly', to '/yearly'. It should stay '/monthly'.

    Marked as helpful
  • Fayaz•380
    @fayax555

    Great Job on the project. There are things you can do to improve your solution even further.

    The clickable area for - and + buttons are tiny. It can be hard for some people to click those icons.

    On smaller screen heights, I'm not able to view lightbox modal's top part (close icon).

    When I open the mobile nav menu and expand the viewport width, the close icon disappears, but the menu still stay open.

    Marked as helpful
  • Fayaz•380
    @fayax555

    I only did the &domain=${ip}, and didn't use &ipAddress=. It seems domain accepts ip address as well, not just domains.

  • Fayaz•380
    @fayax555

    To center the card both horizontally and vertically, remove margin: 0 auto; width: 1440px; and then put display: grid; height: 100vh; place-content: center; to your body. If you give your body width:1440px;, it can give you a horizontal scrollbar when the screen is below 1440px. Use max-width when you have to give a width to containers. But in this project, you don't have to give any widths to your body, simply do the things I've said at the beginning.

    Marked as helpful
  • Fayaz•380
    @fayax555

    https://geo.ipify.org/api/v2/country,city?apiKey=at_RTk0rwxaK8JXPgBj87O7nTL7ug6ZA&domain=8.8.8.8

    I can see your apikey

  • Fayaz•380
    @fayax555

    It's better to use getStaticProps when the api data doesn't change often since it is much faster than getServerSideProps. But I guess it doesn't matter much here, since you cleverly hide the loading state with a nice animation.

    Marked as helpful
  • Fayaz•380
    @fayax555

    Give height: 100vh; display: grid; align-items: center to your body. To make the image fit the container, give your image max-width: 100%, then give some padding (for all sides) to your div.

  • Fayaz•380
    @fayax555

    One way is to move the box out of the card. check these links. I've edited some of your code.

    live
    repo

  • Fayaz•380
    @fayax555

    Good job on the project. One thing I noticed was the cart is buggy because you used unstable value prev.length for your id which was used as the key prop. Instead generate random ids using a library like nanoid

  • Fayaz•380
    @fayax555

    Giving outline: none to focus states reduces accessibility for keyboard navigators. If you must remove the outlines, provide an alternative way for keyboard navigation.

  • Fayaz•380
    @fayax555
    const Controls = (props: Props) => {
  const { searchValueHandler, searchedValue, searchedResult } = props;

  return (
    <div id="controls" className={classes.controls}>
      <Search
        searchValueHandler={searchValueHandler}
        searchedValue={searchedValue}
        searchedResult={searchedResult}
      />
      <Filter />
    </div>
  );
};

    const Controls = (props: Props) => (
  <div id='controls' className={classes.controls}>
    <Search {...props} />
    <Filter />
  </div>
)

    Do you know these two are equivalent?

    Also, you can make use of absolute imports with typescript. I add "baseUrl": "." to my compilerOptions, and then I would import with import { countriesDataType } from 'data/country-data-types' instead of import { countriesDataType } from '../../../data/country-data-types'

    Btw, great job on the project. It seems you've put a lot of effort on this one.

    Marked as helpful