Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found
Not Found

All comments

  • @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

    0
  • P
    matbac85 530

    @matbac85

    Submitted

    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:

    • min-width and max-width.
    • element size in general
    • responsive in general

    I must admit that it's not easy to do "pixel perfect" without figma files.

    and HTML structure:

    • How do you build a form with inputs like these and avoid using div tags?

    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:

    • turn the form inputs into type="number"
    • add a check in your validator to throw an error if input is a string
    • (more complex) find a way to validate the month input so it can accept 1-12 OR the month name. That could be tricky because of the variations: Jan, January, etc..

    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?

    0
  • Shiwansh 50

    @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

    0
  • @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

    0
  • @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

    0
  • Shiwansh 50

    @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

    1
  • @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

    1
  • @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.

    0
  • KamRan 370

    @comendrun

    Submitted

    • Semantic HTML5 markup
    • CSS custom properties
    • Flexbox
    • Mobile-first workflow
    • React - JS library

    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

    0
  • @mseidel819

    Posted

    looks good! I have a list of tiny things that I'll share:

    1. 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} />}

    2. 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.

    3. 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

    0
  • @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

    1
  • Yazdun 1,310

    @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

    Github user search app

    #react#react-testing-library

    1

    @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

    1
  • @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.

    0
  • @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

    0
  • @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>`
    0
  • @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.

    0
  • @mseidel819

    Posted

    I like the creativity on the button colors!

    a couple accessibility fixes:

    • Change <div class="continer"> to <main class="continer">
    • Change <div class="attribution" role="footnote"> to <footer class="attribution">
    • I think you can also get away with changing <h2 class="heading">How did we do?</h2> to an <h1> element.
    • on line 12 of your index.html, in , you can get rid of type="text/javascript".

    Other things you can look into:

    • when you hover over the number buttons and the submit, cursor should be a pointer. cursor:pointer.
    • when you hover over submit, the background should change to white, and the text should change to orange.

    Marked as helpful

    1
  • @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

    0
  • @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

    0
  • dania 530

    @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!

    1. The hover effect on the number buttons should be orange. easy fix :)

    2. 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.

    3. try changing <div class="card"> to <main class="card"> to fix an accessibility issue.

    Marked as helpful

    1
  • @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">

    0
  • @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 got rid of your max-width in the body. This way, your card stays centered even if the screen is way bigger than 1440px.

    -I added a media query for super small screens, so the card isn't cut off.

    Marked as helpful

    0