Skip to content
OverviewSolutions
4
Comments
36
P
Christopher Adolphe
@christopher-adolphe

All comments

  • Emmilie Estabillo•5,600
    @emestabillo
    What are you most proud of, and what would you do differently next time?

    Hey everyone! 👋 It's been a minute! 😊

    I built this project incrementally during the holidays, and took the opportunity to practice Typescript and explore GSAP integration with the app router. To further challenge myself, I used Contentful for managing the project listings and implemented Google Maps on the contact page. It took a little longer than expected to finish, but the process proved to be more valuable than the end result.

    What challenges did you encounter, and how did you overcome them?

    Most of the challenges I encountered were from Typescript. This was the first time I used GSAP with the app router, and I tried to keep the animations on the component files as much as possible, so much so that I was thinking I was creating non-essential files just to support the animation feature. Feedback would be appreciated on that regard, and on any other improvements including accessibility.

    I had to host the fonts since Spartan is not available on Google anymore, and made minor adjustments to the logo to match the design. I also increased the base font to the standard 16px.

    Thanks and looking forward to hearing your thoughts!

    EDIT: From the screenshot, it looks like I followed an outdated design. Will push improvements.

    Arch Studio multi-page site with Next.js, GSAP, and Contentful CMS

    #contentful#gsap#next#typescript
    4
    P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @emestabillo 👋

    You did a great job on this challenge. The result is clean with just enough animation. 🙂

    I had a look at your codebase and I really like the folder structure that you've chosen for the components directory. It makes it easy to navigate through them 👍

    You've also shared some interesting resources in your README.md, thanks a lot for that.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @tinuola 👋

    You did a great job on this challenge. 👍 I had a quick look the code base and here's small feedback:

    • You could simplify the interface of the ProductCard.vue component by declaring only a product props instead of each field of a product as follows:
    defineProps({
  product: Product;
});

    This will enhance readability.

    <ProductCard
  v-for="product in products"
  :key="product.name"
  :class="{ selected: product.selected }"
  :product="product"
/>

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @Samuel-Amaro 👋

    You did a great job on this challenge. 👍 I had a quick look at the application as well as the code base and here's my feedback:

    • The buttons for the dropdown menus are too small (4px by 20px) and as a result, this reduces the both touch and click accuracy. For better user experience, the recommended size of icon buttons should be in a range of 42 to 72px.
    • I think the Content component file contains too many sub components. You could have moved ColumnBoard, TaskList and CardTask in dedicated component files under the components directory. These components indeed compose your content but each of them have different responsibilities which is why I think good to separate them. However, this one is only a personal preference 😉 I know React gives you the freedom of organising your folder structure any way you want.

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @fontainm 👋

    You did a great job on this challenge. 👍

    I had a look at your code and here are a few things that might help you with your class names and the structure of your markup:

    • I get the impression you tried to use the BEM methodology to write your CSS but you did not applied it 100%. You have defined .notifications as a Block which has the .notifications__header and .notifications__list as its Elements. This is good 👍. However, the .notifications Block has other Elements inside it which you chose to target differently. To be honest, it's not a bad thing as such but if you'd like to go the full BEM way, here's a suggestion for the class names and markup:
    <div class="notifications">
  <header class="notifications__header">
    <h1 class="notifications__title">Notifications</h1>
    <span class="notifications__count">{{ countUnread }}</span>
    <button type="button" class="notifications__btn">Mark all as read</button>
  </header>

  <ul class="notifications__list">
    <li class="notifications__item">
      <img class="notifications__avatar"/>

      <div class="notifications__content">
        <div class="notifications__sender"></div>

        <span class="notifications__time"></span>

        <div class="notifications__body"></div>
      </div>
    </li>
  </ul>
</div>

    With the above markup, you will end up with the following CSS:

    .notifications { ... }

.notifications__header { ... }

.notifications__title { ... }

.notifications__count { ... }

.notifications__btn { ... }

.notifications__list { ... }

.notifications__item { ... }

.notifications__item--unread { ... } // This will be your `Modifier` to style an unread item

.notifications__avatar { ... }

.notifications__content { ... }

.notifications__sender { ... }

.notifications__time { ... }

.notifications__body { ... }

    As you can see, if you follow the BEM methodology, your final CSS is completely flat, i.e you'll not have selectors like .notifications__list .notification .message { ... }

    • One last thing, use semantic elements where needed. For example, the Mark all as read should be a <button> instead of a <div>

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @abedfetrat 👋

    You did a great job on this challenge. 👍

    One small thing I have noticed, given I have added 3 new tasks, when I drag the last task from the bottom of the list and drop it at the top, it gets marked as completed. I'm pretty sure this has something to do with the region of the task that's clickable to toggle as active/completed. If you initiate the dragging from this region, when you'll drop the item, the toggling is also triggered. But strangely, I notice it is only happening when dragging an item from the bottom to the top.

    Framer Motion looks really cool. I want to try it on my next project. I've always used CSS modules with React but when I see how clean you've been able to keep those styles within your components, I think I should styled components a try as well.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @MuriWolf 👋

    You did a great job on this challenge. 👍 The markup, styling and component composition are well done. 👌

    One easy fix to get rid of these accessibility report errors/warnings is to set a value to the lang attribute of the <html> tag like so <html lang="en"> in your public directory.

    I have been trying to do various actions in the application and I have noticed that when I reply to a comment, my reply is not added right away. I had to add another reply then both of them appeared. I'm not sure if it's because your REST API is hosted on Heroku and it is responding slowly but you might want to double check that as it hinders the user experience.

    In case it's your REST API which is responding slowly, there is a way you could give a faster feedback to the users. Right now, you've written the CRUD operations of your component in a pessimistic update style, meaning; you are making the calls to the REST API and then you are updating the UI. You may give the user the impression that application is faster by doing the opposite, i.e; you will update the UI first and then make the calls to REST API. This is an optimistic update. 😉 However, when doing so, you will need to keep a reference of the previous state so that if ever an error occurs while doing the update, you can safely revert to that previous state. Below is an example of what an optimistic update looks like in code:

    // 1)Keeping a reference of previous state
const previousComments = [ ...this.comments ];

// 2) Updating the UI
this.comments = [ ...this.comments, this.newComment ];

try {
  // 3) Calling the endpoint to post the new comment
  const response = await fetch('some-endpoint-url', this.newComment);
} catch (error) {
  // 4) Reverting to the previous state if an error occurs
  this.comments = [ ...previousComments ];
  // 5) Giving a feedback that an error occurred
  alert(`Sorry, an error occurred while adding new comment: ${errror}`);
}

    The above code snippet is not Vue.js specific but I think you'll be able to implement it somehow.

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @Shady-Omar 👋

    You did a great job on this challenge. 👍 Here are a few things that I have noticed and that you might want to check in order to improve your solution.

    HTML:

    While the page you built is good, you could improve it by using HTML semantic elements in the following ways:

    • For the navigation, you could wrap the <ul> element in a <nav> element like this:
    <nav>
  <ul>
    <li>
        <a href="#">Home</a>
    </li>
    <li>
        <a href="#">New</a>
    </li>
    <li>
        <a href="#">Popular</a>
    </li>
    <li>
        <a href="#">Trending</a>
    </li>
    <li>
        <a href="#">Categories</a>
    </li>
  </ul>
<nav>
    • The <a> element is also missing for your navigation.
    • Try as much as possible to refrain from duplicating HTML elements on your page because this hinders the maintainability of your code. You can use the same markup for the navigation on both mobile and desktop and adapt the styles by using CSS media queries.
    • You could replace the <div class="top-content"> and <div class="bottom-content"> by a <section> element.
    • For the "New" section, you could replace the <div class="top-right"> as well as it children <div class="text"> elements by an <article> like this:
    <article class="new-post">
  <h2>New</h2>

  <article class="new-post__item">
    <h3>Hydrogen VS Electric Cars</h3>
    <p>Will hydrogen-fueled cars ever catch up to EVs?</p>
  </article>

  ...
</article>
    • You could replace the <div class="topic"> by an <article>.
    • The "Read More" looks like a <button> element but it is in fact an <a> element that will allow the user to navigate to a page where he/she will have access to the whole post. Therefore you should probably define a CSS rule that you can apply to an <a> element so that it looks like a button. 😉
    • Instead of using CSS classes to handle responsive images, you could use the <picture> element together with the <source> element to achieve same like below:
    <picture>
  <source media="(min-width: 1110px)" srcSet="assets/images/image-web-3-desktop.jpg" />
  <source media="(min-width: 768px)" srcSet="assets/images/image-web-3-mobile.jpg" />
  <img src="assets/images/image-web-3-desktop.jpg" alt=alt="web3" />
</picture>

    CSS:

    • In general try to give meaning names to your CSS classes. This will not only help you as well as those you collaborate with you on a project to understand much faster what a particular CSS rule should be used for but will also help to organise your styles better.
    • Try to keep the level of nesting to a maximum of 3 levels deep as this hinders the reusability of your CSS rules. In other words, when you are overly specific, you will have to increase the nesting even more if you want to override any particular CSS rule. The rule of thumb is to define your generic rules first and then define more specific ones for other use cases.
    // DON'T ☝️👎
main .content .top-content .top-right .text h3 {
  margin-bottom: 1rem;
  font-weight: 800;
}

// DO 👌👍
h3 {
  margin-bottom: 1rem;
  color: hsl(240, 100%, 5%);
  font-size: 1.25rem;
  font-weight: 800;
}

.article__item h3 {
  margin-bottom: 0.5rem;
  color: hsl(36, 100%, 99%);
  font-size: 1rem;
}

    RESPONSIVE:

    • On the tablet viewport, the content looks squeezed and there is a huge white space under the "The Bright Future of Web 3.0" section. It would look better if you set this top section in a single column layout on this viewport.

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @Dave 👋,

    You did a great job on this challenge. 👍 I had a great time playing the game too. 😉

    I saw you question about the issues you are having with dependency array of React's useEffect hook. I had a quick look at your code for the Board.js component and here are a few things that I have noticed which might be why you are facing those issues. I'll give you some rules/guidelines that I use myself when I work with the useEffect hook and I'll try to keep it simple so that you'll know where to go from here.

    Rule 1: Don't ignore the warnings about the dependency array

    • We get warnings when React sees we are doing something wrong. You should take a step back and carefully review your implementation of the useEffect hook.

    Rule 2: Know the data types of the dependencies being passed to the array

    • This is where most trouble come from. We should carefully check the data types of the different elements being passed to the dependency array. If the dependencies are primitive data types (i.e: string, number, boolean, null, undefined) we don't have much to worry as they are compared by value. But if they are reference data types (i.e array, object, function, date), the useEffect hook will compare them by reference (i.e their pointer in memory) and each time the component renders, we create a new reference to those dependencies and as the useEffect hook detects they are different, it reruns hence resulting in an infinite loop. 😱 Hopefully, React provides us with tools to work around this problem; they are the useMemo and useCallback hooks. Consider the useEffect below which was extracted from your Board.js component:
    React.useEffect(() => {
    if (gameType !== "CPU") return;
    if (marker !== turn) {
      renderSquareChoice(computerMove(squares, marker));
    }
  }, [gameType, turn]);

    renderSquareChoice is a function inside your component and you are using it inside useEffect, so React prompts you that it needs to be added to the dependency array. Now, you add it as a dependency, you get an infinite loop because renderSquareChoice is a function meaning it is a reference data type. To fix this, you need to wrap the renderSquareChoice with useCallback like so:

    const renderSquareChoice = useCallback((square) => {
    if (status || squares[square]) return;
    const nextSquares = [...squares];
    nextSquares[square] = turn;
    setSquares(nextSquares);
  }, []);

    And you will be able to add it to useEffect as a dependency without causing an infinite loop. NOTE: In the above code, useCallBack also has a dependency array, you might need to add status and squares as dependencies.

    • There are cases where we don't need to pass the entire object or array as a dependency to useEffect. For example, if my component has a person state/prop but my effect is only dependent on the age property of that person state. All we need to do is to pass this age property to useEffect like below:
    const [ person, setPerson ] = useState({
  name: 'Chris',
  age: 36,
  role: 'guest'
});

useEffect(() => {
  // some side effect that needs to run whenever the age of `person` state is mutated
}, [person.age]);

    So we are not passing the entire person state which an object and therefore a reference data type because each render of the component would create a new reference for the person state. Instead, we pass only the age property which is a number and therefore a primitive data type. I illustrated a simple case here but for complex cases the useMemo hook would be more appropriate. This feedback is already very long, I suggest you to read more about useMemo.

    Rule 3: Use the functional variant of the state setter function inside useEffect

    • Whenever useEffect is dependent on a state which is mutated inside it, you should use the functional variant of the state setter function. Taking another example extracted from the Board.js component:
    // Update total scores
  React.useEffect(() => {
    if (status === null) return;
    const newScore = { ...score };
    newScore[status] += 1;
    setScore(newScore);
  }, [status]);

    This useEffect should be refactored as follows:

    // Update total scores
React.useEffect(() => {
  if (status === null) return;
  
  setScore((prevScore) => {
    const newScore = { ...prevScore };

    newScore[status] += 1;

    return newScore;
  });
}, [status]);

    By doing so, our useEffect is no more dependent on score state and hence it isn't needed in the dependency array.

    I know that's a lot to read as feedback but I tried to keep it as lean as possible. useEffect is a big topic in itself. I also got these issues while tackling the Coffeeroasters challenge with React (still in progress 😉). The more you practice, the better you'll get at it.

    I hope this helps you as a starting point.

    Here are some resources from Jack Herrington that helped me:

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @Briuwu 👋

    You did a great job on this challenge. 👍 Here are a few things that I have noticed and that you might want to check in order to improve your solution.

    • View plans and How we work are links that are styled as buttons. So your markup should be <a href="#">VIEW PLANS</a> instead of <button>VIEW PLANS</button>
    • In the navigation bar and the footer, How we work, Blog, etc these should be anchor tag as well.
    • There is a small issue in intro section on tablet. The image seems too large for this viewport width.
    • In the Footer.jsx component, you could also leverage on the map() method to generate the links by adding them to an array like you did in the Info.jsx component.

    src/data/footerLinks.js

    export const footerLinks = [
  {
    menuName: 'Our Company',
    items: [
      {
        title: 'How we work',
        link: '/how-we-work',
      },
      {
        title: 'Why insure ?',
        link: '/why-insure',
      },
      {
        title: 'View plans',
        link: '/view-plans',
      },
    ]
  },
  {
    menuName: 'Help me',
    items: [
      {
        title: 'FAQ',
        link: '/faq',
      },
      {
        title: 'Terms of use',
        link: '/terms-of-use',
      },
      {
        title: 'Policies',
        link: '/policies',
      },
    ]
  }
];

    src/components/Footer.jsx

    import { footerLinks } from '../data/footerLinks';
import { Link } from 'react-router-dom';

const Footer = () => {
  return (
    //...
    <div className="footer-container">
      {
        footerLinks.map((menu, index) => (
          <div className="footer-all" key={`menu-${index}`}>
            <h4 className='footer-title'>{ menu.title }</h4>
            {
              menu.items.map((item, index) => (
                <Link to="{item.link}" className="footer-info" key={`link-${index}`}>{item.title}</Link>
              )
            }
          </div>
        ))
      }
    </div>
    //...
  )
}

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @MoodyJW 👋

    You did a great job on this challenge. 👍 The component composition is well done. 👌

    The only thing I have noticed is that the <body> tag is inside your root AppComponent and in the rendered index.html page you now have a <body> tag nested in another <body> tag which is unfortunately not valid. I understand you did this to implement the theme switching feature. Fortunately, you could resolve this with the help of Renderer2 from @angular/core module. By injecting this service and the DOCUMENT into your AppComponent class, you will be able to add/remove the theme class on the <body> element the "Angular way" without having it inside the root AppComponent template file. Your AppComponent should look something like this after doing this change:

    import { Component, OnInit, Inject, Renderer2 } from '@angular/core';
import {DOCUMENT} from "@angular/common";

/**
* Your other imports, component decorator etc
*/
export class AppComponent implements OnInit {
  theme = window.matchMedia('(prefers-color-scheme: light)').matches
    ? 'light-theme'
    : 'dark-theme';
  user!: User;

  constructor(
    @Inject(DOCUMENT) private document: Document,
    private renderer: Renderer2,
    private usersService: UsersService
  ) {}

  ngOnInit() {
    this.renderer.addClass(this.document.body, this.theme);
    this.usersService
      .getUser('octocat')
      .pipe(first())
      .subscribe((user) => {
          this.user = user;
    });
  }

  toggleTheme() {
    const bodyElem = this.document.body;
    const isLightTheme = bodyElem.classList.contains('light-theme');

    if (!isLightTheme) {
      this.renderer.removeClass(bodyElem, 'dark-theme');
      this.renderer.addClass(bodyElem, 'light-theme');
    } else {
      this.renderer.removeClass(bodyElem, 'light-theme');
      this.renderer.addClass(bodyElem, 'dark-theme');
    }
  }

  userReturned(user: any) {
    this.user = user;
  }
}

    However, with this change, the currentTheme argument passed to the toggleTheme() method would no more be required and therefore, the ThemeToggleComponent would no more need to emit a custom event. Even better, you could move the toggleTheme() method from the root AppComponent to the ThemeToggleComponent since it is the one responsible of this feature. The root AppComponent would thus be only responsible of initialising the user property.

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @Alucard2169 👋

    You did a great job on the overall for this challenge. 👍 I really like the subtle hover effect on the navigation links 👌. Here are a few things that I have noticed and that you might want to check in order to improve your solution.

    • It might be a good thing to have a skip to content link that redirects the user directly to main content is ever they are using assistive technology to visit this landing page.
    • In the vr section, the left and right paddings are not necessary for desktop as they offset the content while in the design, the content of this section is aligned with the next one.
    • In the creation section, I'm not sure if is a good option to use the images a background image as they seem to be part of the content rather than just decorative.
    • On tablet, the <header> is quite huge, maybe you should consider reducing the height and as for the main content, I think that there is ample space to display the creation section in a 2 column layout.

    I hope this helps.

    Keep it up.

  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @Reallyvane 👋

    You did a great job in completing this challenge. 👍 The component is faithful to the design. 👌 Below are a few things that I have noticed and you might want to check in order to improve your solution.

    • When I look at this component, I would expect to see a <form> element with <input type="radio"/> elements for the different ratings and a submit <button> in the HTML markup. However, it is not the case in your solution. While the component seem to be working fine for the general users, those with assistive technology might struggle to interact with it. This is why I would suggest you to use HTML elements that are built to take user inputs when it is the case. For example <input type="radio"/> or <input type="checkbox"/> to choose from a list of options instead of <div> elements. This will also simplify things on the JavaScript side. 😉
    • I also noticed that you have set a default rating value of 3 but at the moment, there is no visual cue that communicates that to the user. You should add the active class by default on the corresponding <div class="rating-btn"> element. Again, if it was a <input type="radio"/>, you would have used the checked attribute to achieve that. On the other hand, if you don't wish to set a default rating value, you would then have to add a check in the onSubmit() function like this:
    let stars_score;

function onSubmit() {
  // Returning early if stars_score is not set
  if (!stars_score) {
    return;
  }
  
  card_content_1.classList.add("hide");
  score.textContent = stars_score;
  card_content_2.classList.remove("hide");
}

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @tiffanicodes, 👋

    You did a nice job in completing this challenge. 👍 I have recently completed the same project and I think I can give you some tips on how you could improve your solution or at least think of a different approach.

    • I see that you have built the main content section using the grid-area property from CSS grid. While this is not a bad approach, it is not a flexible solution. Here are 2 resources that will surely help you come up with a more efficient layout. Travis Horn - Responsive grid, CSS-TRICKS - Responsive Grid
    • In order to render the hero title in black and white, you can apply the mix-blend-mode property. It can have different values depending on the result you want to get, in the case of this challenge, the exclusion value does the job. 😉 Read more about it here.
    • The spacings in the footer section need to be reviewed as at the moment it looks wider than the header and main sections. I would suggest that you add a wrapping <div class="container"> element inside the different sections of your page to which you then apply a max-width. This will resolve the problem as well as on the location page.
    • On the CSS side, I would suggest that you refrain from using id as selectors to style elements as this hinders reusability.

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @brasspetals,

    You did a great job on this challenge. The parallax effect is perfect. It really accentuates this feeling of depth on the page. 👌👌

    As for the tabbed content, it is still flashing at the moment. I tried to toggle the fadein and fadeout classes manually on the <img /> element and I noticed that the flashing occurs when the fadeout class is applied. The image is still visible with this class applied to it. Maybe that's where you need to have a closer look. 🤔 I recently watched a video from Kevin Powell about animating from display none. Maybe that could help also you.

    I also noticed that on large viewports (1600px and above) there are like to 2 black stripes on the each side of the page and it is caused by the fact that the <div class="container"> element is restricted by a max-width. Was this intentional ? 🤔

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @Briuwu 👋

    You did a great job in completing this challenge using React. 👍 Below are a few things that I have noticed and you might want check in order to improve your solution.

    • On large viewports the <Social /> and <Overview /> components align to the left side. To correct this, simply apply margin-left: auto; and margin-right: auto; to them or you could apply display: flex; on the <main> element.
    • On tablet maybe it would be better to keep a 2 column layout instead of 3 for the <Social /> and <Overview /> components. At the moment, you have one card overflowing to the next line which creates an unnecessary whitespace.
    • As for the CSS, it is not as messy as you might be thinking. 😉 You could certainly improve on the organization of your partials. One thing I have noticed though, it would be better to move the compiled CSS in a different directory which you could call css and keep your SASS files in another directory called sass. This way when you deploy the finished project, only the compiled CSS is bundled in the deployment. However, if this is already being taken care of during the build process, please ignore this comment. 😉
    • You could also consider using CSS modules stylesheet, which aim at scoping the style to the component. For example your <Dashboard /> component would have its own Dashboard.module.scss file. I think this also greatly helps in keeping your CSS organized. You can read more here

    I hope this helps.

    Keep it up.

    Marked as helpful
  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @toriola998, 👋

    You did a great job in completing this challenge using Vue. 👍 Below are a few thing that I have noticed and you might want to check in order to improve your solution.

    • On larger viewports the overall content on the pages stretches horizontally thus making the images appear huge. I see that you have used padding for the spacing on each sides. I would suggest you use a wrapping <div> element inside each <section> to which you could apply a max-width.
    • For the cart button, it would be better to use a <span> for the cart count because a <p> element cannot be the child of a <button> element.
    • When I tried to add the same product twice to the cart, it was added as a new item in the cart instead of showing x2. The increment and decrement feature is also missing on the product details page.
    • The overall composition of your page structure could be improved by moving the <NavBar />, <header />, <main> and <TheFooter /> components to the App.vue root component. This would reduce repetition in the different views component.
    • The social icons in the footer should have links to redirect the user to the respective social media website.

    I hope this helps.

    Keep it up.

  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @RMK-creative,

    Thank you. 😉

    Yeah feel free to go through the git repo, you'll hopefully find something useful. As for GSAP, I recently started using it as well and I can tell you that you can do awesome stuff with that.

    As for the interactive map, this morning I did some changes to move the mapbox access token to an environnement variable on Netlify. I must have done something wrong. Thanks for notifying. I'll check it out.

    Best regards.

  • P
    Christopher Adolphe•620
    @christopher-adolphe

    Hi @francisldn 👋

    You did a good job on completing this challenge with CSS grid and React. 👍 I am currently working on the same challenge and I'm only left with a few minor refinements before posting my solution. I wanted to review your solution so as to compare how we approached this challenge and if possible share some tips with you. 😉 Below are few things I have noticed and you might want to check in order to improve your solution.

    • Rendering the main heading in 2 colours is surely the part that makes you scratch your head when you see the design file for the 1st time. 😆 But I can tell you that you can achieve that by using a single CSS property called mix-blend-mode. It can have different values depending on the result you want to get, in the case of this challenge, the exclusion value does the job. Read more about it here. You can apply it to your .heading class like this:
    .heading {
  mix-blend-mode: exclusion;
}
    • Along the same vein, I think that it would be better to use an <h1> element for the Modern Art Gallery text instead of a <div> and then in the main content section use <h2> elements for the other heading.
    • The arrow for the Our location link is larger as compared to the design. Moreover, it is a link so this means that it should be an <a href="/location"></a> element instead of a <div>. Since you are using React and React Router, it makes sense to use the <Link to="/location"> component for this. To get the arrow to the correct size, you could refactor this part like this:
    <Link className="btn" to="/location">
  <span className="btn__text">Our Location</span>
  <span className="btn__icon">
    <img className="arrow" ref={locationImageRef} src={arrowRight} alt="" />
  </span>
</Link>

    You can then use the .btn__icon class to style the part of the link with the golden background and the arrow. Also note that you can leave the alt attribute of the <img/> element blank since this is a decorative image.

    • For better accessibility, please use a <main> element to wrap the main content of the page and a <footer> element for the bottom part of the page.
    • On large viewports, the content is stretching horizontally thus making the images of the main content look larger as compared to the design. You could correct this by adding <div className="container"> element to wrap the different sections of the page and applying a max-width to it. In the design, the maximum width of the page is 1110px.
    • On the overall, the way you have approached the layout of the whole page with CSS grid is not bad but I think it lacks of flexibility and might be difficult to maintain in the long run. Here are 2 resources that will surely help you come up with a more efficient layout. Travis Horn - Responsive grid, CSS-TRICKS - Responsive Grid
    • In the main content, you are serving the desktop image for smaller viewports. In the design file, some of these images become in portrait mode on smaller viewports. You can have responsive images by using the <picture> and <source> elements in conjunction with an <img/> element to serve the right image on corresponding viewport widths. Read more here.
    • Please check the styling for the heading as some of them are not picking the correct font family.
    • The social icons look larger as compared to the design and the links to redirect the user to the respective social media website are missing.

    I hope this helps.

    Keep it up.