search

cncf / landscapeapp

🌄Upstream landscape generation application
https://landscapes.dev/
Apache License 2.0
255 stars 125 forks source link

Stop cards with second_paths being repeated within card mode group #852

Closed milanlakhani closed 1 year ago

milanlakhani commented 1 year ago

If an item appears twice in the landscape using the second path variable, this stops it from being repeated in a card mode group (currently it would be repeated multiple times even within a card group).

The item card still shows up in all its different groups when grouping is used.

CC: @michaelmoss @awright @GeriG966 @danielsilverstone-ct @abhi0469

netlify[bot] commented 1 year ago

Deploy Preview for landscapeapp ready!

Name Link
Latest commit ffb653c7835cfb4a692d3b9d30bc823420cf6428
Latest deploy log https://app.netlify.com/sites/landscapeapp/deploys/63740ed88c1ce500081ca050
Deploy Preview https://deploy-preview-852--landscapeapp.netlify.app
Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

milanlakhani commented 1 year ago

Hi @AndreyKozlov1984 , I'm back from my holiday now, hope you are well. Feel free to let me know any improvements that I can make if it would help :)

AndreyKozlov1984 commented 1 year ago

What is the desired behaviour, though, is it that an item appears only once in a card mode ?

milanlakhani commented 1 year ago

What is the desired behaviour, though, is it that an item appears only once in a card mode ?

Once per group when using 'Grouping' or yes once if there is no Grouping. (Just because in our use case we were using second_path and the exact same item was appearing multiple times).

AndreyKozlov1984 commented 1 year ago

Ok, we also have an embedded script, it should share this behaviour as well. EmbedPageRenderer.js , and also the complexity is o(n*n), not sure how many items we can have in this itemsWithDuplicates, but we have over 1K items in the cncf.

AndreyKozlov1984 commented 1 year ago

I'm going to apply fixes myself and then to merge.

AndreyKozlov1984 commented 1 year ago

Even better, that code should be executed only on the server side - either during compilation of the embedded page, or when an api is called.

AndreyKozlov1984 commented 1 year ago

I fixed this in the main branch

milanlakhani commented 1 year ago

Thanks :)