shalvah / oldtweets.today

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

Added the collapsible by years now. #34

Closed MichaelGee closed 5 years ago

MichaelGee commented 5 years ago

Closes #31

oneEyedSunday commented 5 years ago

Yh. Like I said. This is just a personal style choice. Feel free to disregard. Just something minimalist. The colours are getting rather obvious on the screen. The blue against the background isn't so subtle

On Mon, 19 Aug 2019, 11:24 Michael Ekwere, notifications@github.com wrote:

@MichaelGee commented on this pull request.

In docs/index.html https://github.com/shalvah/oldtweets.today/pull/34#discussion_r315140195 :

@@ -95,6 +95,25 @@ min-width: 120px; }

  • .year_button {
  • background-color:#0d47a1;
  • display:block !important;
  • width: 50%;

I dont...understand. I thought we wanted a button that shows the year?

— You are receiving this because you commented. Reply to this email directly, view it on GitHub https://github.com/shalvah/oldtweets.today/pull/34?email_source=notifications&email_token=AHDNOO2C6PXB5EHOUPFYA2DQFJYGFA5CNFSM4IMULSBKYY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOCB5JCAQ#discussion_r315140195, or mute the thread https://github.com/notifications/unsubscribe-auth/AHDNOO6VGW46J4XUQBFHJPTQFJYGFANCNFSM4IMULSBA .

shalvah commented 5 years ago

This looks interesting. For a UI change like this, we need more screenshots.

Also, when results are loaded, which is the default view (collapsed or expanded)?

Don't forget to remove that snapshots folder.

@oneEyedSunday, this LGTM, but then, my eyes aren't the best.😅 If you think it can look better, then it's best to wait till this is merged and then send in that as an improvement.

shalvah commented 5 years ago

Btw, @MichaelGee, before/after screenshots means "how did the frontend look before you added this change vs after?" Not before/after entering a username. This means both screenshots should be of the page in "results" state.

oneEyedSunday commented 5 years ago

Hahaha. I've been looking at bootstrap blue for a long time. I'd do just that if I get to it. This issue had been on my plate for a while.

On Mon, 19 Aug 2019, 12:01 Shalvah, notifications@github.com wrote:

This looks interesting. For a UI change like this, we need more screenshots.

  • How does it look expanded?
  • How does on mobile (expanded and collapsed)?

Also, when results are loaded, which is the default view (collapsed or expanded)?

Don't forget to remove that snapshots folder.

@oneEyedSunday https://github.com/oneEyedSunday, this LGTM, but then, my eyes aren't the best.😅 If you think it can look better, then it's best to wait till this is merged and then send in that as an improvement.

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/shalvah/oldtweets.today/pull/34?email_source=notifications&email_token=AHDNOO6OZRVZASBYFRA3R6DQFJ4PXA5CNFSM4IMULSBKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOD4SQRYA#issuecomment-522520800, or mute the thread https://github.com/notifications/unsubscribe-auth/AHDNOO65P5JGHS5M6JOCWMTQFJ4PXANCNFSM4IMULSBA .

MichaelGee commented 5 years ago

So i should send more snapshots right now?

MichaelGee commented 5 years ago

screencapture-localhost-5500-2019-08-19-12_24_06 screencapture-localhost-5500-2019-08-19-12_24_53 screencapture-localhost-5500-2019-08-19-12_26_56 screencapture-localhost-5500-2019-08-19-12_27_12

shalvah commented 5 years ago

Yes, more screenshots. And don't forget to remove the ones you added to the code.

MichaelGee commented 5 years ago

Ok. Done.

shalvah commented 5 years ago

Nice! Last questions:

MichaelGee commented 5 years ago

Expand isnt default. Once the page loads, its closed, so all u have to do is click on the button to reveal the Tweet for each year.

shalvah commented 5 years ago

Does the cursor change on hover so it's obvious the years are clickable?

You didn't answer this?