CodingGarden / seedling-school-01-frontend-project

For your first project, you will build a frontend website with HTML, CSS and JavaScript. The website will communicate and integrate with a 3rd party Web API.

MIT License

106 stars 14 forks source link

Non-Earthling News Project Submission #1

Open CassandraGoose opened 5 years ago

CassandraGoose commented 5 years ago

Project Name / Title

Non-Earthling News

Your Name / Title

Cassandra/ummmheck

Project Description

Get the latest news about our friends from outer space, sourced from all over the web. With easy sorting, filtering and searchability, you will be able to find all the latest alien information you desire.

https://github.com/CassandraGoose/non-earthling-news

What 3rd Party Web API do you plan to use?

https://newsapi.org/docs

Which of the following describes you:

[x] YouTube Subscriber
[x] Twitch Follower
[ ] Patreon Patron
[ ] Superchat Donor
[ ] Streamlabs Donor
[x] Coding Garden Moderator

CassandraGoose commented 5 years ago

Submitted this for review! Please let me know if this description will suffice and I will start working. Thanks a ton!

w3cj commented 5 years ago

Hey @CassandraGoose This looks good overall. The only thing I would update is the How will users interact with your web site? section.

Part of the requirements are that there is a landing page with a description of your app, and then the user can click a button to go to the app (search page in your example). https://github.com/CodingGarden/seedling-school-01-frontend-project#what-you-will-build

Update that section and then you can get started on wireframing and coding!

CassandraGoose commented 5 years ago

Thanks a ton - Updated! I will get started now.

CassandraGoose commented 5 years ago

Update: Here are my wireframes (super basic):

w3cj commented 5 years ago

Looking good! Thanks for sharing. This will definitely help others when they start their project. Feel free to post screen shots of your app as you build it as well!

CassandraGoose commented 5 years ago

Another update - turns out that the developer version of the API I am using only gives snippets of the articles instead of the whole article. To work around this, I'm going to have a 'Read more' at the end of each snippet that links to the original article (which is provided by the API).

Always good to dig deep into your API before committing to using it or be prepared to do some work-arounds.

CassandraGoose commented 5 years ago

First page scaffolded:

CassandraGoose commented 5 years ago

Second page scaffold + initial JS:

w3cj commented 5 years ago

Looks great!!

CassandraGoose commented 5 years ago

Hey! I have completed my features -> I would LOVE LOVE LOVE to steal a code review and a UI/UX review. No rush. Thanks much, friend!

w3cj commented 5 years ago

UI Feedback

Things I like

The landing page tells the user exactly what this app does.
Loading indicator is really cool! Looks like a crecent moon.
Font choice - I like that you use 2 fonts, one for the title and one for the body
Overall style makes it feel like a newspaper! I like that.
Screen resizing on desktop looks great! I would remove the medium breakpoint and only have the small and large.

To improve

Show the loading indicator when switching between sort values
It looks like the Nasa / Area 51 buttons are content filters, and not new source filters. Seems a bit out of place from the rest of the buttons. Maybe have it on the next line? Or a different color?
It would be nice if the filter buttons changed color / size to indicate that they are active
When filter buttons are clicked, they do not keep the sort in mind. This is OK if the filter buttons had some sort of visual indicator.
The search box is ignored after changing sort value.
On mobile, the breakpoints don't seem to work.

Nice to have

Use animations when list items are entering / leaving

Great work overall!! Code review up next!

w3cj commented 5 years ago

Code Review

main.html

Line 16 - search box should be in a form instead of a div.

index.js

Be consistent with semicolons
The formatDate function is a bit redundant. Can reduce the code to just:
```
dateP.textContent = new Date().toDateString();
```

main.js

put the url in a variable
same fetch twice in the getNews function. Did you mean to add a query param?
nonBannedArticles function might be re-named to removeBannedArticles or filteredArticles
A few missing semicolons. Be consistent!

You might try using a regular expression instead of looping over the banned words.

const nonBannedArticles = (articles) => {
const bannedRegexp = new RegExp(bannedWords.join('|'), 'ig');
return articles.filter((article) => {
return !article.title.match(bannedRegexp);
});
};

In the addArticlesToPage function, you can use the content element you created, instead of selecting at the bottom of the function.

If you have the time, watch the video to see more feedback not listed here. Also, take a look at the checklists I created in this repo.

CassandraGoose commented 4 years ago

Updates have been made - thanks for going through everything!