search

shalvah / oldtweets.today

"On This Day" for Twitter #TwitterThrowback
http://oldtweets.today
113 stars 24 forks source link

Snapshots #33

Closed MichaelGee closed 5 years ago

MichaelGee commented 5 years ago

Added the snapshots in an img folder.

shalvah commented 5 years ago

Hey, I don't mean add the screenshots to the code. Add them to the PR. You can paste or upload a picture into GitHub's comment box.

The idea is so that the reviewer can see the actual effect of your PR before merging it.

shalvah commented 5 years ago

I don't think you understood the issue #30 properly.

It refers to the view that is shown after results are fetched. Old tweets are listed by year. But the problem is, if I want to see my tweets from 2013, I'd have to scroll down all the way from 2019.

The task is to modify the header shown for each year so that:

  1. The header is clickable
  2. Clicking the header hides all the tweets in that year.
  3. Clicking the header again expands all the tweets in that year.

Let me know if you need further clarification. Would have uploaded an image to show what I mean, but I'm on mobile.

shalvah commented 5 years ago

Aha, here's perfect example of the effect I mean: https://getbootstrap.com/docs/4.3/components/collapse/

(Obviously we're not using Bootstrap.)

MichaelGee commented 5 years ago

Ok.. I think I wanna put the file into fragments.. modularize it. It's a little confusing to read.. (I'm sorry, I am still learning how to code and also version control.. padorn my little mistakes or time wasting sometimes. 🙂)

shalvah commented 5 years ago

No problem; you've not done badly thus far, and it's not an urgent issue.

It's good practice to read the contribution guidelines (the CONTRIBUTING.md file) first before opening a PR. I think GitHub auto-suggests this to you when opening your first PR.

I'm going to close this PR so you can open a new one when you're ready. I've updated the description of the issue #30 so it's clearer. Let me know if you still don't understand.

Regarding modularizing: (refactoring ) that's a separate task, so I will not merge a PR that does both together. One PR per task. Also, I really don't have time for reviewing a refactoring PR, so if you send in such, I won't look at it for a long time. I suggest you focus on the current issue, then we can discuss refactoring later.

The task is mostly a styling one. I'm not a frontend person, but I'll say you need to attach a class to the tweets container for each year and then cause the appropriate container to be hidden/shown when the corresponding year header is clicked (you'll also need to modify the look of the year headers so it's obvious they're clickable).