Schindler Dumagat

Latest comments

• 

  P

  Emmanuel Lopez•410

  @EmLopezDev

  Submitted 19 days ago

  What are you most proud of, and what would you do differently next time?

  I am most proud of the fact that I actually finished it and got it to work. There were moments I felt like giving up like when trying to figure out how to generate the random password based on the inputs, but I stuck it out and did it. I also am proud of taking the extra steps to add disabled states to the buttons just for a better user experience.

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

  • Adding custom styles to the input range was bit of a challenge but luckily I found a good resource to help.
  • Randomizing the characters for the password was a challenge because it involved a fair amount of logic, however through a bunch of trial and error I got it to work.

  What specific areas of your project would you like help with?

  I definitely would like feedback on the JavaScript.

  I cleaned up the logic as much as I think I could but I am sure there is more I could have done or a better approach.

  Would like feedback in general on the file but for a bit more specificity these two logics:

  const randomCharacter = () => {
      finalPW = "";
      const checked = isChecked(checkedOptions);
      while (finalPW.length < lengthValue) {
          const randomIndex = randomNumber(checked.length);
          const randomCategory = checked[randomIndex];
          const categoryValues = passwordCharacters[randomCategory];
          const randomCharAt = randomNumber(categoryValues.length + 1);
          const randomCharacter = categoryValues.charAt(randomCharAt);
          finalPW += randomCharacter;
      }
  };
  
  const finalPwStrength = () => {
      const checkedLength = isChecked(checkedOptions).length;
      if (lengthValue < 15 && checkedLength === 1) {
          changeGauges(1, "very weak");
      } else if (lengthValue < 15 && checkedLength === 2) {
          changeGauges(2, "weak");
      } else if (lengthValue < 15 && checkedLength > 2) {
          changeGauges(3, "medium");
      } else if (lengthValue >= 15 && checkedLength === 1) {
          changeGauges(2, "weak");
      } else if (lengthValue >= 15 && checkedLength === 2) {
          changeGauges(3, "medium");
      } else {
          changeGauges(4, "strong");
      }
  };
  

  

  Password Generator

  #sass/scss

  1

  00
  

  P

  Schindler Dumagat•340

  @schindlerdumagat

  Posted 3 days ago

  For the randomCharacter function which I believe is for generating the password, its good enough. But for the finalPwStrength function which I believe is for evaluating the password generated, I have some concerns.

  Upon reading the finalPwStrength function code block, I noticed that your evaluation for the password's strength is based only on the values set by the user through the slider and checkbox input elements but not on the actual password itself.

  To explain further, here is a sample scenario I tested with your current solution:

  1. I set the character length to 5 using the slider and I checked the lowercase checkbox only so it should generate password with lowercase characters only. The strength is "Very Weak".

  2. I set the character length to 5 again using the slider and I checked the lowercase and number checkbox only so it should generate password with lowercase characters and numbers only. The strength is "Weak".

  3. Using the values in number 2, I kept generating a password until I get a password with only lowercase characters even though the number checkbox is checked. The strength is still "Weak" and I think it should be "Very Weak" because it is the same thing as not checking the number checkbox at all.

  4. Therefore, I conclude that you are not actually evaluating the actual password generated. Instead, you are evaluating it based on the values set by the user using the slider and checkbox input elements.

  Moreover, your isChecked function is returning an array but you use the "is" keyword in naming the function. It is a best practice to start the name of a function with "is" if it is expected to return a boolean value.

  Hope this helps!

  0
• 

  P

  jrybarski•210

  @jrybarski

  Submitted about 1 month ago

  

  Calculator using javascript events

  1

  20
  

  P

  Schindler Dumagat•340

  @schindlerdumagat

  Posted about 1 month ago

  Here are the concerns that I have noticed:

  • It seems like you haven't implemented the design for the tablet view which is in the design file given.
  • There are still some inconsistencies with the design. (font color for the label of tip percentage selection, placeholder text color for the inputs, etc)
  • In your HTML, you should enclose the elements within the body in landmark roles (e.g header, main, footer).
  • Don't use multiple h1 tags in the same page. Use only one.
  • Don't use px for font sizes, use rem instead. By using rem, font sizes will adjust according to user settings.
  • Their are some calculation issues that I have found. Make sure you fill out all the necessary fields before you calculate the result. It seems like you are already starting to calculate even though some fields are still empty. Error messages are not showing when entering an invalid value in the input field (e.g entering 0) and it is also in the design.

  There are still more but this is ok for now.

  0
• 

  Ahsan Ali•180

  @AhsanAli-Dev0

  Submitted 6 months ago

  

  HTML, CSS, BOOTSTRAP, JAVASCRIPT

  1

  10
  

  P

  Schindler Dumagat•340

  @schindlerdumagat

  Posted about 2 months ago

  I see that there are still some visual inconsistencies with the design so I believe this is not done yet but I will still give you some feedback.

  Here are the things I noticed in your code:

  • Add a modern CSS reset to revert most of the styling applied by the browser user agent styles. This helps in styling your elements with ease.
  • Use h1 to h6 elements in order. Don't skip heading levels.
  • You should enclose your elements inside the body element in a landmark role for accessibility improvement.
  • Don't use px for font sizes, use rem instead. Using px for font sizes will not respect user browser settings. Using rem will respect user browser settings.
  • I recommend that you revisit some of the articles and challenges in the learning paths before the JavaScript learning path. Upon checking your solution, it seems like you lack some of the foundations you need to learn.

  0

  Marked as helpful
• 

  Vincin Christmas•170

  @VincinChristmas

  Submitted 2 months ago

  What are you most proud of, and what would you do differently next time?

  I am happy that I am getting more familiar with Sass and JavaScript event listeners

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

  the JavaScript portion was a little challenging.

  What specific areas of your project would you like help with?

  I know that I am not supposed to use styling in the JavaScript, so I will need help with how to change that. I plan to come back and fix this later.

  

  The News Letter Sign Up

  #sass/scss

  1

  00
  

  P

  Schindler Dumagat•340

  @schindlerdumagat

  Posted about 2 months ago

  Here are the issues I saw:

  HTML

  • You should enclose all the html code in the body element within a landmark role. In this case, you can put it in a main tag.
  • I recommend that you use a picture element for the illustration image to be able to download only what is needed based on the device width. Your solution currently download all of the illustration even though you only need the mobile version if you are on mobile view for example.
  • I recommend that you use h2 tag instead of h1 for the form title because the form will only be a part of the page and it will not probably hold the main title of the page.
  • The icon list are just decorative. They don't provide context to the page so an alt attribute with the value of empty string is enough to make it decorative. Do the same thing with the icon image in the success page.

  CSS

  • There are still some inconsistencies with the UI. It seems like the tablet view was not implemented. You also need to fix some parts of UI to closely match the design provided.

  JS

  • As you have mentioned, you have added styling logic to your JavaScript code directly. For you to fix that, you can create classes instead and style them in your CSS. To apply those classes in your elements, you can use methods on your elements such as element.classList.add("className"), element.classList.remove("className"), element.classList.toggle("className") just to name a few. You can learn more about how this code works on the internet.

  1

  Marked as helpful
• 

  Matthew Sheldon•320

  @MatthewNgabirano

  Submitted 2 months ago

  What are you most proud of, and what would you do differently next time?

  Finally, another project is done within two days. Next time am planning to add js on every part of the project I do.

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

  I encountered the challenge of aligning displaying the image on top of the parent so that the rest of the children are below it but I finally got by looking into the code to see where the problem was.

  What specific areas of your project would you like help with?

  I am currently learning js and it would be the great part of the area I would like help with.

  

  Article-preview-component

  #accessibility

  1

  20
  

  P

  Schindler Dumagat•340

  @schindlerdumagat

  Posted 2 months ago

  Upon checking the live site, it seems that this project is not yet finished. I can still see some inconsistencies like the colors, spacings, responsiveness, etc. But I would like to give some feedback on some of the things that I can see right now as incorrect and provide some recommendations.

  Here are my recommendations:

  • It is better for performance to link fonts in HTML head tag instead of link it in the CSS file because it is the HTML file that the browser receives first before anything else.
  • You should put all the contents of your HTML body tag in a landmark role for accessibility reasons. In this particular, you should enclose the contents of the body element in a main tag. You can read more about landmark roles on the internet and see why it is necessary.
  • You should use heading tags in order (from h1 to h6). Don't use heading tags just to make the font size of a text smaller or bigger. You can do that in the CSS instead.
  • Even if you are already learning JavaScript, you should still revisit your HTML and CSS because it seems like you have missed some of the most important concepts and knowledge when working with HTML and CSS.

  0

  Marked as helpful
• 

  P

  Mars - Martiniano Leguizamon•255

  @martinianol

  Submitted 3 months ago

  What are you most proud of, and what would you do differently next time?

  I’m proud of the clean, responsive design and well-structured Sass. Next time, I’d refine accessibility and optimize animations for a smoother experience.

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

  One challenge was aligning the button to the right in the footer. I fixed it by using margin-left: auto instead of justify-self, which only works in CSS Grid. Debugging layout issues with Flexbox helped me understand its behavior better. 🚀

  What specific areas of your project would you like help with?

  Images are not exactly as the prototype but I believe the images provided are not the correct ones...

  

  Responsive Landing Page | HTML, Sass, Flexbox & Media Queries

  #sass/scss

  1

  20
  

  P

  Schindler Dumagat•340

  @schindlerdumagat

  Posted 3 months ago

  Here are my feedbacks:

  • I recommend that you use some CSS methodologies like BEM in styling your elements for better readability and code structure. But you can use other CSS methodologies as well if you want.
  • For the logo, you can use img tag with alt attribute value of "meet" for better accessibility.
  • I think using the article tag for the hero text content is not right since it doesn't stand out when this is moved to another page. And since it doesn't have an aria label, it is just like div. So, it is better to use div instead.
  • It is better to import fonts in the HTML head element instead because HTML is the first resource the browser reads. Thus, it is faster to load.
  • You can further breakdown you styles.scss by creating another scss partial file called _variables.scss or whatever you want to call it and just import it to your styles.scss for better readability and for separation of concerns. You can check the sass documentation on how to implement this. Just some nice tips.
  • Avoid using fixed units like px on your font sizes, use rem instead. Font sizes set in rems adapt automatically to user preferences. I recommend you read this resouce.
  • Avoid styling elements by their ids, use classes instead. Ids have a higher specificity than classes and it would be difficult to change styles later on.
  • I also recommend not to style elements by their tag name for maintainability reasons. For example, you want to change the element used in your html for accessibility reasons, you would have to change it in your CSS as well. You can use class instead for easier maintainability.
  • Its better to use unitless values on line heights as it scales properly according to the font size.
  • Don't use px in media queries, use rem or em instead.
  • I recommend that you read the benefits of using relative units instead of px for you to become wiser in what unit to use. I have seen in your CSS file that you use px in most of the time which is not a good idea.
  • There are also some modern CSS resets on the internet which can check or even add to your code. Just for you to be aware.
  • As for your concern on the images in the hero section, you can check on others code, understand it and then apply it to your code.

  1

  Marked as helpful