Looking to hire developers?
@Skidragon
@Allangui
Submitted
Hi guys !
It would be very nice if you guys could give me some feedback on this challenge :) There are many possibilities of problems on this one lol I did it using Vue 3 and making a localStorage on the user-preference-Theme
Thanks , have fun ! :D
@Skidragon
Posted
Hi Allan, nice job completing the challenge and it being useable! For the small buttons, grid-template-columns: repeat(4, 1fr) and grid-template-rows: repeat(4, 1fr) would have made a 4 by 4 grid then the big buttons at the bottom could have been grid-template-columns: repeat: (2, 1fr). Also the buttons should be button elements rather than div elements, I can't use the accessibility capabilities that buttons have like keyboard accessibility. Anyway three components could be made here to organize it: Button, Display, and RangeInput. Putting the CSS with those components into seperate files would be easier to read through the code and to create better names for the CSS rather than for1, for2, for3 or div1, div2, div3. Material UI has a variant property prop to change the buttons styles like 'primary' or 'secondary'.
Marked as helpful
@yingmo55
Submitted
Currently, I hard coded in a wait time (about 1.5 seconds) between button click by disabling the random button, because repeatedly clicking it doesn't seem to update the quote (the function is called when I tested it.) Is it the APIs' limitation, or the way I implemented the fetch method?
Thanks in advance for taking a look at the project!
@Skidragon
Posted
Hi Yingyi, good job on completing the project! It's kind of a combination of the browser and the API. The API wants to cache the advice for 2 seconds on the server to save extra db calls plus the server will use less resources and most modern browsers wants to cache data as much as possible since its cheaper to get data from the browser rather than browser -> server -> database -> server -> browser. They might be only using redis (key,value) store and might not need a seperate database.
Calling with the same URL might make the browser store the URL as a key and the data as the value { URL: data }. So the solution I did was put a new Date() generated timestamp query param at the end of my URL: https://api.adviceslip.com/advice?timestamp=1647571215979. Last thing, the advice container doesn't need a media query, using width: 100%; and max-width: 40rem or something will do the trick instead of vw. Anyway I did wish they would send to the UI how long they will cache things to disable the button on the UI for the period.
Marked as helpful
@vikyij
Submitted
Thinking about the solution gave me tough time especially when I have to track the selected items and if they match.
@Skidragon
Posted
Hi Victoria, good job at making this and I had fun playing with it. I'm also working on this project and working with Xstate and spawning player instances depending on what the user chooses. Just some ideas that I'm throwing out here: Use a custom component using radio element to create the create game form buttons for accessibility purposes. Xstate.js is simplifying the game making process for me at the moment, essentially I have a game actor and spawn player actors to talk to each other. The game actor state holds the tokens, spawn the amount of players selected, keeps track of player's turn. The player actor tells the game actor that I matched these tokens correctly by storing it in its own state. Also the game actor has different phases like initializingGame -> choosingPlayer -> playerMoves -> checkWin -> choosingPlayer or gameOver. Here is the idea thus far: https://github.com/Skidragon/monorepo-portfolio/tree/memory/apps/memory
@NotKijana
Submitted
I did not find much difficulty with this project, which is very pleasing. This means that I am getting better, also this project only took me like an hour and most of it was styling. I was not sure how I could expand on it but, I was able to come up with adding a search bar. I do want to get more practice having reusable CSS without using a library like bootstrap. I will begin to work on my own global styles sheet.
Thank you for the challenge frontendmentor!
@Skidragon
Posted
Hi Kijana, great job completing the project and adding in a search field! I do have a couple of suggestions, since this project isn't very big I would have made only 3 components: AdviceContainer to replace Box since it describes it better because every element is a box essentially, SearchField, and Footer. If the company expands more on the project then we can break out into more smaller components if need be. Also you could look into react-query, I've used it professionally and its great for fetching data and do mutations (create, update, delete). It even tells the status of the fetching process. Another way to challenge yourself is try to use typescript if you have the time. I recommend Jack Herrington youtube channel to learn from his playlist. I hope this is helpful!
Marked as helpful
I found the overall page designs not so difficult, however i had some issues with the cart and i went off design a bit to implement the stripe api for real payement's, this i found quite difficult and it is not currently working as i would like. any feedback appreciated
@Skidragon
Posted
Hi @cooper, good job on making it full stack. I do have a couple of suggestions, since it's an e-commerce site, SEO is very important, I would look into Next.js because it helps with dealing with SEO, lazy loading images, and even if the user has javascript disabled, they can still see the web page since javascript can build the page on the server rather than client side. Also for the height of the footer, I would use padding with em units instead of height, it adjusts to the content inside the footer rather than a fixed height. I would watch Kevin Powel's em vs rem on youtube. I would make the stepper field input box as disabled and I think there is a type conversion happening because I'm able to add more items then the max. Here is my solution if you want to take a look:
https://www.frontendmentor.io/solutions/nx-nextjs-typescript-graphcms-nestjs-and-styled-components-cPFj72mX9
I didn't do every section since I would need to upload more images/content to the CMS.
@ndgdl
Submitted
First challenge on Frontend Mentor, had a lot of fun :)
I struggled with the responsiveness:
Regarding the fetch request, I was wondering if there is a way to prevent spamming the requests. The easiest way for me was disabling the button with a timeout of 1 sec, but not sure if it's best practice.
Any other feedback would be welcome!
@Skidragon
Posted
Hi Nicolas, for width, I usually set the width: 100% then I set a max-width: 120ch or something. I use rem/em instead of pixels because some devices calculate pixels differently. For fetching requests, disabling it is good enough for the frontend since this API should be taking care of any attacks like DOS (Denial of service). If its not an API that uses caching and throttling the requests, you would need to have your own server to talk between the client and the API.
Marked as helpful
@igbokwe-kosi
Submitted
Any addition or advice is recommended!
@Skidragon
Posted
It looks awesome on the styling and responsiveness. Also liked the variable names and their casing. I would make the input search box not cursor:pointer and just use the default value because it feels like a button to me. Just things to be aware of from a backend perspective: hide the api key on a reverse proxy server which talks between the client and the api in this way you can rate limit and prevent DOS (Denial of Service) attacks unless the API itself does it for you plus I can't steal your api key and make requests in my own app, and in the innerHTML for renderMap function, if the API ever becomes malicious which may never happen in this case, they could add a javascript script in the code (XSS) and make it get information about users' searches though in a more complex app, user authentication information like stealing a JWT from local storage or something. Don't spend too much time on backend stuff if its frontend that you're specializing in and need to get a job quickly.
Marked as helpful
@pernorin
Submitted
For some reason the button dosn't work on Firefox, though refreshing the page loads a new advice. Can anyone figure out why?
@Skidragon
Posted
In firefox and maybe in most modern browsers, it caches the same request like a key so it just keeps going to the browser cache to get the info instead of the api. I added a timestamp query param that gives me a new value (key) everytime at the end of the api endpoint instead of updating the headers to no-cache because it wasn't working in my case. Here is an example: https://api.adviceslip.com/advice?timestamp=1646669540478
Marked as helpful
@catherineisonline
Submitted
Hello, Frontend Mentor community! This is my solution to the Advice Generator app.
I have read all the feedback on this project and improved my code. Due to the fact that I published this project very long ago, I am no longer updating it and changing its status to Public Archive on my Github.
You are free to download or use the code for reference in your projects, but I no longer update it or accept any feedback.
Thank you
@Skidragon
Posted
It looks good though I keep getting a cached response when fetching for new advice. I believe it has to do with fetch since I used axios for mine, and you might have to do what this post suggested: https://stackoverflow.com/questions/29246444/fetch-how-do-you-make-a-non-cached-request
Just noticed its doing it on mine too. I think it might be firefox dev edition browser.
Marked as helpful
@grizhlieCodes
Submitted
Hey all,
Took my time with this one. I think overall it took me 3 weeks with a lot of refactoring.
Key FEATURE (bug) 🤩: When you create a new account it will populate 3 default invoices into the app. For some reason random inline-styling is being added to them and changes their size to something ridiculously small. Just refresh and we're golden. No clue, sorry. Oh yeah, it doesn't always happen. So I can't replicate it on demand.
Feel free to login with: username: [email protected] password: 123456
Some things to points out:
OVERALL... Great project for learning about data-flow, structure and general planning. I wish i spent more time planning everything as I did find multiple issues towards the end. But I'm super happy with this project, I think it's the biggest learning curve I've had (per project) from anything I've done so far.
Any thoughts, suggestions, etc welcome.
Cheerio
@Skidragon
Posted
It looks and animates pretty awesome! The only suggestions I have is the confirm deletion modal could be exited out of when pressing esc and just being more keyboard accessible. Also the fields for creating an invoice could have more validation like the email and having an autocomplete fields for the state and city but MVP wise it's pretty solid. Keep up the good work and grit! I might look at the code base more tomorrow when I have more time. Also I'm doing the audiophile guru one to challenge myself too.
@norman02
Submitted
I got the background working this time. Still having trouble getting drop down menus to line up.
Feedback welcomed!
@Skidragon
Posted
Hi norman02, for spacing list of links between your elements I would look into the lobotomized owl selector * + * since the footer links are kind of squished. Also for your buttons, you can make your buttons grow relative to your text in the button and don't have to use a set width and rem for padding. I would look up em vs rem by Kevin Powell, he does a good explanation. Essentially use em on the padding to set your width and height of your button instead of the rigid width and height properties. Example:
.btn {
display: inline-block;
background-color: white;
padding: 1em;
color: #FF505C;
font-weight: bold;
border-radius: 28px;
}
if you change the font size later for your button in a media query, the width and height of the button will grow with it. Also it will grow or shrink depending on how much text you add into it.
@Romario-Negreiros
Submitted
I would like to know if the methods I used on my javascript, as to insert data on the screen, and to navigate from both pages, using session storage to get data from the previous page, is a good practice, I had no lib or frameworks knowledge, so that's why I used javascript.
@Skidragon
Posted
Hi @Romario-Negreiros, for organizing, I would recommend using PascalCase for your classes to be able to distinguish them faster since that is common practice. Also for your item which I guess is a Country Card? The methods need to tell me more about what the code is doing, not how it is doing it. For example: insertClass in the Item class doesn't tell me much and I would have to search for the class name to understand what it is doing which is very problematic in huge projects.
Here is an example of how I would structure the project:
CountriesList.js
CountryCard.js
home.js //where we can import our classes to interact with each other
CountriesList can have the methods
add(CountryCard)
remove(CountryCard) //We can do this instead of an index because we are pointing to the same object in memory
addMultiple(countryCardsData)
CountryCard (name, population, region, capital, link)
in home.js
const data = await fetch()
const countryCardsArr = [];
loop through data and use it to create the CountryCards and push them into the array
CountriesList.addMultiple(countryCardsArr);
I haven't looked into the session storage of your code but you can also use query params to fetch data in the next page if you need to store an id to get more detailed information about a country.
@justin-xia15
Submitted
This is my first challenge and I'm having trouble getting it to display properly. Everything works fine on my local computer but after hosting it on GitHub Pages, the CSS does not seem to be working. Any help would be greatly appreciated!
@Skidragon
Posted
Hi @justin-xia15, I think the reason why the CSS is not working is because of this line:
<link rel="stylesheet" href="/Stats-preview-card-component/css/styles.css">
it needs to change to this:
<link rel="stylesheet" href="./styles.css">
This is referred to as getting the relative path vs the absolute path.
I see that these folders /Stats-preview-card-component/css/ doesn't exist in your repo. It might exist on your local though.
@tomsky90
Submitted
Hi Guys. any advice on how can I improve my code will be much welcome .
@Skidragon
Posted
Hi @tomsky90, nice job on the dark mode and filtering. I do recommend to setup prettier to format your code to make it look nicer and not worry about formatting code. Also the components need to be simplified more, for example:
const HomePage = ({
error,
setError,
darkMode,
countriesListData,
countriesDataForDisplay,
setCountriesDataForDisplay,
selectValue,
setSelectValue,
inputValue,
setInputValue,
}) => {
return (
<div>
<Form
setError={setError}
darkMode={darkMode}
countriesListData={countriesListData}
countriesDataForDisplay={countriesDataForDisplay}
setCountriesDataForDisplay={setCountriesDataForDisplay}
selectValue={selectValue}
setSelectValue={setSelectValue}
inputValue={inputValue}
setInputValue={setInputValue}
/>
this is like saying in vanilla js land Form(setError, darkMode, countriesListData, countriesDataForDisplay, setCountriesDataForDisplay, selectValue, setSelectValue, inputValue, setInputValue), I as a developer of this re-usable component wouldn't want to deal with. It needs to be more generic of a form. For example:
<Form
isDarkMode={true} //the "is" tells me it's a boolean
data={[]}
onSubmit={() => {
// fetch, set your countries, set errors, and set load here
}}
onInputValueChange={() => {}}
onSelectValueChange={() => {}}
This way you can re-use the component in more than one place.
@hi-reeve
Submitted
hey everyone this is my first time working with geolocation api. map and ip api this is my first and very amazing. please give me feedback on this one
@Skidragon
Posted
Hi @zynth17, seems like everything works well! The only thing is instead of setting the component widths to have like 70% and 30% like the card and search bar, I believe it would be better to give them a max-width and set them to have a 100% width because they are getting a little too small. Also the card with the ip information text is overflowing it's container. I wish I can post photos here.
Marked as helpful
@ruona88
Submitted
Comments Please
@Skidragon
Posted
Hi Ejovi, looks pretty good. :) I would suggest looking into em vs rem for CSS and the dangers of using fixed heights from Kevin Powell from Youtube. Also next time use a mobile first approach for your media queries, these things will save you more time on bigger projects. For the main content section, I would suggest a main element in place of a section element to communicate better that this is your main content.
@AmanyadavFD
Submitted
@Skidragon
Posted
Hi @AmanyadavFD, pretty good job on the responsiveness and styling, for the html, I would recommend the main element instead of a section element since you put that as your main. Also I think it's kind of redundant doing classes like main__heading-span. A .purple-accent class would make more since for that. Also I don't see the point of putting a .secondary-heading class when the h2 communicates that already. If I was going to put a class there, I would communicate what the h2 is referring to. For media queries, I think you took the desktop first approach, I would go with the mobile approach first. Use min-width instead of max-width to do a mobile first approach, you can then use $tablet and $desktop variables to place into here instead of this. bp doesn't give me much info.
@media only screen and (max-width: $bp-small) {
order: 1;
border-radius: 0 0 1rem 1rem;
text-align: center;
}
In summary to improve, I would work on more descriptive and concise class names and organization plus the mobile first approach to help other devs understand the code and save more time for yourself in the future.
@Skidragon
Submitted
I would like feedback on how accessible my site is for example keyboard users. Also I would like to know how you guys controlled the grey block svgs which I used pseudo classes. Another thing is how anyone moved their image off screen a little bit, I used negative margins for that task which I think isn't the best because I had to use overflow: hidden for the mobile then unset it on bigger screens. If there is any SCSS tips or accessibility tips let me know. :)
@Skidragon
Posted
One last thing, how did you guys use filter to create the orange overlay on the keyboard image or was it a different property?
@nicoPuegher
Submitted
Is there a better way to solve the color approach for each hover?
@Skidragon
Posted
Good job on making it so similar! I just made my background Learn More Button value to none and let the card background to take care of it for me.
.card__cta:hover {
background: none;
box-shadow: 0 0 0 0.2rem var(--white);
color: var(--white);
}
Here's mine if you want to take a look, I should finish the details but I got other things to do: https://github.com/Skidragon/sd-3-column-preview-card-component-main
Btw, I would replace the button elements with the link element since the user would usually go to another page when clicked on.