search

mensatt / frontend

2 stars 2 forks source link

Hide unavailable occurrences #10

Closed stormofice closed 9 months ago

stormofice commented 9 months ago

Unavailable dishes (on a day) should be indicated a bit more intuitively in the future. Hiding them is still fine for now, as the view does get very messy when multiple dishes are replaced on a day.

netlify[bot] commented 9 months ago

Deploy Preview for mensatt-nuxt ready!

Name Link
Latest commit cb7bec066e6050bee0010e52819a50d529e09dd6
Latest deploy log https://app.netlify.com/sites/mensatt-nuxt/deploys/6516f54950554a0007b58de7
Deploy Preview https://deploy-preview-10--mensatt-nuxt.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 configuration.

maanex commented 9 months ago

Two style changes:

  1. Please use const instead of let if the variable doesn't get re-assigned elsewhere which is the case here.
  2. Please don't use Array.concat as it is notorious for causing unpredictable results. Preferred alternative is array spread operator. a.concat(b) -> [...a, ...b]
stormofice commented 9 months ago

Sounds good, should be fixed now 🫡

maanex commented 9 months ago

Thanks! General question, would it make sense to add the new logic to the filters.filterOccurrences() method itself instead of filtering it beforehand? Is there a scenario where we wouldn't want outdated occurrences filtered out?

stormofice commented 9 months ago

I had similar thoughts, but worried about how easy it would be to change (for example) the way unavailable dishes are displayed. If we were to go through with the idea of greying them out "in-place" instead of hiding them, it would (currently) not fit that well into the filters.filterOccurrences(..) function.

maanex commented 9 months ago

That's a good call. Let's keep it as is then 👍 Feel free to merge any time