@Mistie-rious
Submitted
for some reason my buttons have outlines, not sure why also the react router dom isn't needed, I realized that later
Looking to hire developers?
@mseidel819
@Mistie-rious
Submitted
for some reason my buttons have outlines, not sure why also the react router dom isn't needed, I realized that later
@mseidel819
Posted
the <button/>
component comes with a border by default. To get rid of it, in your css file, you can add:
button {
border:none;
}
Or you can add the border:none
to both your .circles
and .submit
classes. I'd pick the first option because it's less code.
Marked as helpful
I have to say I'm proud to have reached the end of this challenge. It's not perfect, but it's not bad :)
I still have some difficulties with CSS concepts such as:
I must admit that it's not easy to do "pixel perfect" without figma files.
and HTML structure:
Also, placing this button above this line has given me new white hairs.
I'm just starting out in Vue.js, but I need to improve for my internship at the end of June. If you have experience with this framework, please don't hesitate to give me feedback or advice.
@mseidel819
Posted
Looks nice! I'm giving myself a crashcourse on Vue, and I'm glad I found your app to help guide me!
I noticed that your validation doesn't account for string inputs. Some solutions might be:
type="number"
as for your question "How do you build a form with inputs like these and avoid using div tags?", My intuition says its ok to use the <div>
like you are. I've seen it done in credible tutorials. Does the HTML accessibility checker on here give you an error?
@shiwanhs05
Submitted
Hi, Everyone. This is the third challenge I completed on frontendmentor. With every challenge, I try to improve my methodology, thought process, responsiveness, and code quality. Feedbacks are something I am looking for. Your feedback and suggestions would be much appreciated.
Thank You
@mseidel819
Posted
Looking at your semantic markup--
You want to consider screen readers. If you couldn't see your page, and a robot was describing it to you, would it be clear?
First thing, I think you should change your <div class="add-to-cart-btn">
from a div
to a button
. That will signal to the computer that you have a button that is supposed to do something.
For your header elements, think about how you rank the most important things. This Can line up with font sizes, but not always. In this case, I think the first thing that the viewer needs to know is the product name- this should probably be your <h1>
. after that, the <h4 class="segment">Perfume</h4>
could probably be <h2>
. The prices could probbly end up being <span>
elements. I don't think prices are considered semantic elements. (but don't quote me on that.)
Marked as helpful
@lorenacrincon
Submitted
This challenge helped me a lot to reinforce some basic React tools.
@mseidel819
Posted
Very nice! I think you could move the isSubmitted
and selectedValue
logic into App.js
. That way, you can keep the two components more separated.
return isSubmitted ? (<Ranking rankingHandler={rankingHandler} /> ) : (<ThankYou selectedValue={selectedValue} /> );
I think this will clean up your files a little bit. As I type this, I realize you will have to use some props in your components, and some handlers for the scoring. I'm happy to talk more about it if you have questions or you're interested.
Marked as helpful
@CodinGitHub
Submitted
Any feedback is welcome!
@mseidel819
Posted
If you want to take this further, and practice rendering HTML inside your .js file, you could render your notification boxes dynamically. maybe you create a data.json
file with all of the posts. something like:
[
{name: "CodingTube",
img: "./images/codingtube.png",
content:"left the group Chess Club"
},
{name: "Matt",
img: "./images/matt.png",
content:"followed you!"
},
...
]
You'll have to structure it in a way that works for you, and fits every use case for the different types of posts.
Then you could target the div that contains the posts, use a map function, and insertAdjascentHTML to fill the boxes up. some pseudo-code:
import Data from "./data.json"
const data = Data; (redundant?)
const commentContainer = document.getElementById('comments');
data.forEach(post => {
commentContainer.insertAdjacentHTML('beforeEnd",
`
<div class="post">
<img class="img" src=${post.img} alt="" />
<div>
<p class="text">
<span class="name">${post.name}</span>
<span class="group">${post.content}</span>
<span class="status not-read"></span>
</p>
<p class="time">${post.date}</p>
</div>
</div>
`
)
});
I'm not using my editor for this, so please excuse any syntax errors....
This way, you can reduce some of the repeated code. Also, if you wanted to add or remove, some posts (very similar to a real-world use case), you can just update the data file, instead of the HTML.
Marked as helpful
@shiwanhs05
Submitted
Hi Everyone, I am a beginner in front-end website development. I try to write clean code. Any feedbacks are most welcome regarding the ui or codebase.
Thanks! Have a great day :)
@mseidel819
Posted
I think you could add a media query so that the height: 100vh
on the body only appears on large screens. It is causing some issues with the box shadow overlapping content on mobile.
Another little bonus: you could add cursor: pointer
to your .continue-btn
class to signal that the button is clickable(even though its not connected to anything for this challenge). also, you could add a :hover
effect on the button for more interactivity. Maybe a slightly lighter color, or an opacity?
.continue-btn:hover{
opacity: .8;
}
Marked as helpful
@ifeanyiih
Submitted
feedback would be appreciated
@mseidel819
Posted
This looks good! I like the way you implemented the "time posted" feature. I was just using dates, so I want to look into how you did that.
When I upvote the 1st level comments, they aren't sorting. Try moving your sort function around to another place. From what I think I see, you initialize comments
with a sorted array, but they never sort again after that first time. For example, what I did in my app was to sort the comments in the return function:
{
{ comments_map.sort((a, b) => {
return b.score - a.score;
}).map(comment => {
return (
<Comment
key={comment.id + comment.user.username}
comment={comment}
comments={comments}
setComments={setComments}
currentUser={currentUser}
/>
)
})
}
}
now, each time this component re-renders due to an updated state, it will sort the comments accordingly.
Marked as helpful
@jhigginson
Submitted
Any feedback is welcome!
@mseidel819
Posted
I like how you handled the theme toggler. Mine ended up only being able to switch 1,2,3,1,2,3. I like that yours can go from 1 to 3 right away.
The only potential help I can give is within your themes.js
file. Instead of export { themeOne };
, you can just add the export
directly to the const declaration:
export const themeOne = { "--main-text": "white", "--main-bg": "hsl(222, 26%, 31%)", "--toggle-key": "hsl(6, 63%, 50%)", "--toggle-bg": "hsl(223, 31%, 20%)", "--toggle-h": "rgba(249,108,99,255)", "--keypad-bg": "hsl(223, 31%, 20%)", "--screen-bg": "hsl(224, 36%, 15%)", ...
All of your imports will get to stay the same, too.
@comendrun
Submitted
I used this opportunity to practice my ReactJs knowledge and also CSS Custom-Properties. I managed to do all CSS responsiveness with only CSS Custom Properties and I really liked it. it felt amazing to only change a couple of things and Voila! now it works both in mobile mode and also desktop mode. To see how you can add code snippets, see below:
I definitely would want to cut off some of my repeated code and also reach the goal with much fewer lines of code. Maybe I have to try using useRef, but that was not something I intended for this challenge.
@mseidel819
Posted
nice! someone shared this with me, so I'll pass it to you. In order to get rid of the little scroll button on your number input, add this to your css: `/* Chrome, Safari, Edge, Opera */ input::-webkit-outer-spin-button, input::-webkit-inner-spin-button { -webkit-appearance: none; margin: 0; }
/* Firefox */ input[type="number"] { -moz-appearance: textfield; }`
Marked as helpful
@rnastoff
Submitted
@mseidel819
Posted
looks good! I have a list of tiny things that I'll share:
Instead of 2 short circuit statements in App.js, you can also write 1 ternary operator:
{submitted ?<ThankYouModal rating={rating} /> : <RatingModal onHandleRating={handleRating} onHandleSubmit={handleSubmit} rating={rating} />}
when you pass props to a component, something you can do (especially if you end up using props a LOT in a component) is to destructure the props at the beginning: `const RatingButton = ({onHandleRating, num}) => {
const handleClick = () => {
onHandleRating(num);
}
}
in this example,
propsgot destructured to
{onHandleRating, num}. Now, you don't have to include
props.` in front of them each time you use them.
continuing in Rating.js, another way to handle the click event (i think) is to put onClick={()=>onHandleRating(num)}
. The const handleClick
is good practice though for when you have a large function that you need to use. This function is so small that you could do either.
Marked as helpful
Hi, I am learning React, so your suggestions are more than welcome. Thanks!
@mseidel819
Posted
Your solution is very clean and easy to read! Looks great.
The only suggestion I have deals with the jsx that renders the tip buttons. Its totally unnecessary, but you might be able to save yourself some lines of code if you figure out a way to get rid of the repeated code. The only real differences for each button are the percent values inside. Maybe something like:
const percentArray=[5, 10, 15, 25, 50];
...
percentArray.map(percent=>{ return ( <button onClick={() => setTip(percent)} className={tip === {percent} ? 'active tip-btn' : 'tip-btn'} > {percent}% </button> )
so I added the percent values to an array that I could map over. Then I replaced the hard-coded values with the variable in the map function. (full disclosure: theres a 95% chance my brackets and parentheses are wrong in my example. upside down smiley face)
Marked as helpful
@Yazdun
Submitted
Hey everyone ! I normally would've done this challenge with Vanilla JS, But I've started learning react testing library recently and I wanted to write some tests against a simple app so I picked up this challenge.
Feedbacks are appreciated
@mseidel819
Posted
I love all the animations! I searched my account, and some weird stuff happened with the spacing on my profile info. search for random users and you'll see what I mean. I'm running out of time to work this morning, but I'll take another look after work today. Its an otherwise very nice experience!
Marked as helpful
@SaintShegs
Submitted
Feedback welcome Suggestions are also welcomed
@mseidel819
Posted
It looks like every listing has the new
and featured
tags on them. Try creating a conditional for those tags so that it only displays if the json data marks those listings as "new" and "featured". Something like this:
<div className='job-item-details-header'> <span><h4>{item.company}</h4></span> {item.new && (<span>New!</span>)} {item.featured && (<span>Featured</span>)} </div>
What I've added is a short-circuit conditional that will only render each of those span
elements if the corresponding value is true. You can also add this to the green border on the left of each job card. I believe it only shows up on a job if it's featured.
@Dev-Tron
Submitted
@mseidel819
Posted
I like your solution for light/dark themes! I'm going to study up on how you implemented that.
Something you can do to organize your components is to put all of your styled components in their own folder, and export them out. so, instead of one SearchApp.js
, you would have SearchApp.component.js
, and SearchApp.styles.js
.
Some of your .styles.js
folder might then look like :
export const Container = styled.div
display: flex;
flex-wrap: wrap;
margin: 0 auto;
export const Header = styled.header
width: 100%;
`
export const Wrap = styled.div`
display: flex;
justify-content: space-between;
width: 100%;
`
export const Title = styled.h1`
color: ${Props => Props.theme.titleColor};
font-size: xx-large;
transition: all 0.5s ease;
``
and then in your SearchApp.component.js
, you would import it all:
import {Container, Header, Wrap, Title} from "./SearchApp.styles";
Marked as helpful
@abishekbardewa
Submitted
@mseidel819
Posted
This looks great! I like how you added the "rate again" functionality.
Another way you can switch between components is very similar to what you've done, but it might involve less lines of code:
Instead of checking for truthiness in each individual component, add a ternary to App.js. You'll also be able to get rid of the isSubmitted
prop from each component.
`<main> { isSubmitted ? <Thankyou rating={rating} handleSubmit={handleSubmit} /> : <Rating handleRating={handleRating} handleSubmit={handleSubmit} />
<Footer /> </main>`@gioblasco
Submitted
Any advices on how to improve the styling organization? And is the components' organization for React good enough or could it be more fragmented?
@mseidel819
Posted
This looks great! I think your code looks nicely organized, so my comments are just another way of doing it.
I think you could get rid of your container.js and put all of that code into App.js. (and move your container.sass file accordingly, too). That way, you get rid of the middle-man by storing your state inside App.js. It is one less folder, and this extra code inside App.js wont be enough to clutter.
When I did this challenge, I made a wasRatingSubmitted: false
inside state. Then, I used that to determine which component to show. The submit
button will then have an onClick that will change wasRatingSubmitted
to true
.
{this.state.submitted===true ? <ThankYouState rate={this.state.submittedRating} /> : <ActiveState onRateSubmit={(rate) => this.handleRateSubmit(rate)}/>
This way, you'll have both your wasRatingSubmitted
and submittedRating
inside state incase this project was to expand and now you need to know if the user submitted a rating somewhere else.
@BiscuitCandy
Submitted
Feedback is appreciated!
@mseidel819
Posted
I like the creativity on the button colors!
a couple accessibility fixes:
<div class="continer">
to <main class="continer">
<div class="attribution" role="footnote">
to <footer class="attribution"> <h2 class="heading">How did we do?</h2>
to an <h1>
element.
, you can get rid of type="text/javascript"
.Other things you can look into:
cursor:pointer
.Marked as helpful
@Ugboaja-Uchechi
Submitted
Hi everyone, any feedback on how I can make my code neater and better is appreciated.
@mseidel819
Posted
I put in a super long url, and my shortly code was cut off. Could you add a horizontal scroll for any urls that are bigger than my screen? I think that all you'll need is overflow-x: scroll
or overflow-x: auto
inside the css for the component that shows the long text.
Another bonus would be to allow more than one shortened url to show up. So Instead of assigning the input to a string, you could push it into an array, then map over that array to render each url into its own container. Tools you can use:
const urlDisplay= document.querySelector(#containeryoureaddingto);
urlDisplay.insertAdjacentHTML(afterbegin, '<div>containers you'll add with each item of array</div>)
const urlArray=[ ];
urlArray.map((url)=>{})`
I made up variables, so you'll have to change them as they apply to your app. Experiment with how you'll use the map function. I think there's a couple places you could fit it.
Marked as helpful
@dhananjaysahu79
Submitted
Feedbacks are welcome. What do you think about the code quality?
@mseidel819
Posted
This looks great!
a couple super easy fixes that will get rid of some accessibility and HTML issues:
make sure to put alt
tags on all of your images. Because your images in this app are more for style, and not necessarily "important" to the users functional experience, you could add alt=""
inside your images.
the id
tag can only be used once. It is a very specific property. try changing your id=btn2
to class=btn2
.
I think if you change your <div className="App">
to <main className="App">
you will clear up another accessibility issue.
You need an <h1>
tag somewhere (but only 1). Maybe try changing <h2>How did we do?</h2>
to <h1>How did we do?</h1>
(and then style it so that it looks the same as before).
I THINK this should clear everything up. Then, this app will be looking really clean!
Marked as helpful
@adimidania
Submitted
Hello dear community! I just finished this challenge and I would really appreciate your feedback 🥰 Happy coding and keep it up 💪
@mseidel819
Posted
This looks great!
The hover effect on the number buttons should be orange. easy fix :)
There may be an issue with your <img>
component towards the bottom of your html file. try adding a />
to the end of it, instead of just a >
. your issue report says u
> <h3 id=selected-result
></h3>`.... I think that's what the issue is.
try changing <div class="card">
to <main class="card">
to fix an accessibility issue.
Marked as helpful
@LucasBispoMenezes
Submitted
hello solution made with javascript html and css, any tips to improve my techniques are welcome
@mseidel819
Posted
Way to finish this! It looks great!
In order to fix some accessibility issues, try making these modifications:
change <div class="card">
to <main class="card">
change <h3>
to a <h1>
. You may need to restyle the text so that it looks the way you had it before. The browser wants to see a level one header (h1), and because this is the biggest thing, it would make a good one. Make sure you only have one H1.
-change <div class="attribution">
to <footer class="attribution">
@Lakorthus
Submitted
Nice challenge to put on practice your skills with background images!
feedbacks are welcome!!
@mseidel819
Posted
I'm going to send you a pull request. ( I think I know how)... There are a couple things I changed: -I don't think you need the grid display in the .container class. I commented it out.
-I added a media query for super small screens, so the card isn't cut off.
Marked as helpful