Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

WIP: Implement date sorting in timeline #125

Closed
wants to merge 1 commit into from

Conversation

Mikescops
Copy link
Member

@Mikescops Mikescops commented Jan 17, 2020

This is a first draft for the timeline date separator.

fix #45

image

@Mikescops
Copy link
Member Author

@jancborchardt Need your input for the expected design

@skjnldsv
Copy link
Member

Super awesome!! Very great job @Mikescops 🚀 🤗

@skjnldsv skjnldsv added 2. developing Work in progress enhancement New feature or request feature: timeline Related to the timeline section labels Jan 18, 2020
@rullzer
Copy link
Member

rullzer commented Jan 18, 2020

wow! Awesome work!

@jancborchardt
Copy link
Member

jancborchardt commented Jan 18, 2020

Aaaaawesome! :) Some feedback as said:

  • Have some more whitespace above the date dividers (except the first one)
  • Let’s only start with month dividing first, so "January 2020", "December 2019" etc
  • The month can be bold, but the year can be normal weight for less focus. So put the year in a span and set font-weight: normal to override it. :)
  • At some point we should have it sticky, but only if it doesn’t put a full white bar on the top and it looks nice overlapping the photos. iOS/macOS Photos is the benchmark here. :) It’s better to not have it sticky than to have it sticky and not look good.

@Mikescops
Copy link
Member Author

image

@skjnldsv skjnldsv added this to the 1.1.0 milestone Jan 19, 2020
@Mikescops
Copy link
Member Author

Mikescops commented Jan 20, 2020

@skjnldsv i completely changed the way we load element because it was a source a confusion and could break at any time because it was empirical way of counting the height of the elements within the grid.
At some point we may want to inject other elements in the grid such as big images / maps / text ... and we can't predict their ratio.
I'm not so fond of using grid for sure, i think in the old gallery the original ratio images were more nice looking (@jancborchardt your opinion here). Maybe we should go for flexbox.
For now i basically take the window size and the computed size of the grid and check if the viewport is 256px (row size) to the bottom and load the next batch of images.
It works really nice on my computer, but you may want to test on other browsers / platforms.

Copy link
Member

@skjnldsv skjnldsv left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@skjnldsv i completely changed the way we load element because it was a source a confusion and could break at any time because it was empirical way of counting the height of the elements within the grid.

So you just removed the entire virtual scroller implementation ?
This is a no go for me. Open the timeline with 100k images and see your browser crash :)

@Mikescops
Copy link
Member Author

@skjnldsv how this could crash? This version is even more efficient in term of memory..

@Mikescops
Copy link
Member Author

I think i understand now what you did, but we should improve it, or do it differently

@jancborchardt
Copy link
Member

So what are the next steps here @skjnldsv @Mikescops? :)


I'm not so fond of using grid for sure, i think in the old gallery the original ratio images were more nice looking (@jancborchardt your opinion here).

That’s a separate issue from this pull request :) see discussion and options at #145 (TL;DR let’s first go with this view that e.g. iOS and Android Photos as well as Instagram use.)

@skjnldsv
Copy link
Member

That’s a separate issue from this pull request :) see discussion and options at #145 (TL;DR let’s first go with this view that e.g. iOS and Android Photos as well as Instagram use.)

I think Corentin was more refering of the technical css grid implementation :)

@Mikescops
Copy link
Member Author

Yes

Signed-off-by: Corentin Mors <corentin.mors@dashlane.com>
@Mikescops Mikescops force-pushed the feature/implement-timeline-date-grouping branch from 5c9b9a6 to a1bc036 Compare March 10, 2020 11:10
@Mikescops
Copy link
Member Author

Spent 4hours on it and i can't find a way to do it properly without changing the whole grid system.

@aomader
Copy link

aomader commented Mar 11, 2020

This write-up about the Google Photos Web UI and how they achieve their photo placement might be helpful, as they tackled the same problem and solved it pretty well I would say.

@skjnldsv
Copy link
Member

Especially this part: https://medium.com/google-design/google-photos-45b714dfbed1#247b

While most users will have thousands of photos in their library the screen can usually only fit a few dozen.
So, instead of placing every photo into the page and keeping it there, every time the user scrolls we calculate what photos should be visible and make sure they are in the document.
For any photo that used to be in the document but is no longer visible we pull it back out.
While scrolling the page there are probably never more than 50 photos present, even as you scroll through tens of thousands. This keeps the page snappy at all times and prevents crashing the tab.
And, because we group photos into segments and sections we can often take a shortcut and detach entire groups instead of each individual photo.

Same as we do, conceptually at least :)

I'll read about it more in details, I gave a few quick looks at this article before, and need to find the time to dive into it a bit further!
Thanks for sharing!

@Mikescops
Copy link
Member Author

I think the way we get the image list can be sort of blocker, because it's not so flexible

@skjnldsv
Copy link
Member

I think the way we get the image list can be sort of blocker, because it's not so flexible

How so? What do you have in mind? :)

@skjnldsv
Copy link
Member

@rullzer rullzer removed this from the Nextcloud 19 milestone Apr 14, 2020
@simonspa

This comment has been minimized.

@skjnldsv

This comment has been minimized.

@tacruc
Copy link

tacruc commented Sep 25, 2020

tangbc/vue-virtual-scroll-list#49 (comment)

Any new as the the issue is closed?

@skjnldsv
Copy link
Member

skjnldsv commented Sep 30, 2020

Any new as the the issue is closed?

I guess we'll either have to rely on another lib or fix our own implementation :(

I found one react implementation that works super well! https://github.com/jamiebuilds/react-gridlist
But the syntax of react is really something I cannot fathom 😁

@Mikescops
Copy link
Member Author

Shall we recode it for Vue?

@skjnldsv
Copy link
Member

skjnldsv commented Oct 2, 2020

Shall we recode it for Vue?

Feel free to do so. Such lib would most likely gain lots of interest as there is none! :)

@Mikescops
Copy link
Member Author

😅 yeah sure 😅

@dassio
Copy link
Contributor

dassio commented Oct 6, 2020

sweat_smile yeah sure sweat_smile

hope this server features can help you : #466

@Mikescops
Copy link
Member Author

FYI, I did the lib, should be soon integrated 😅 https://www.npmjs.com/package/vue-virtual-grid

@jospoortvliet
Copy link
Member

FYI, I did the lib, should be soon integrated sweat_smile https://www.npmjs.com/package/vue-virtual-grid

you're a champion! 🦸

@Mikescops
Copy link
Member Author

This has been reworked in #468

I'm closing this PR as it is obsolete.

@Mikescops Mikescops closed this Oct 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
2. developing Work in progress enhancement New feature or request feature: timeline Related to the timeline section
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Date separators
9 participants