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

feat(directory): Add front page categories #4560

Merged

Conversation

connoratrug
Copy link
Contributor

@connoratrug connoratrug commented Dec 12, 2024

What are the main changes you did

  • added images
  • add card component
  • replace current with image-cards

note: to test, enable frontpage by setting initialLandingpage.enabled to true

Todo

  • add links/ actions, ie what happens when a user clicks a cards
  • link to config, tie to or extend some/all stuff to config ( can be separate pr if needed )
  • add test

How to test

  • explain here what to do to test this (or point to unit tests)

Checklist

  • updated docs in case of new feature
  • added/updated tests
  • added/updated testplan to include a test for this fix, including ref to bug using # notation

@connoratrug connoratrug changed the title feat: Add front page categories feat(directory): Add front page categories Dec 12, 2024
@connoratrug connoratrug marked this pull request as ready for review December 13, 2024 11:45
Copy link
Member

@esthervanenckevort esthervanenckevort left a comment

Choose a reason for hiding this comment

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

Het gaat de goede richting op, maar er is iets waar ik niet helemaal mijn vinger op kan leggen wat waardoor het er nog niet optimaal uitziet. Ik denk dat het komt omdat de afbeeldingen en de tekst behoorlijk groot zijn in vergelijking tot het voorbeeld in het ontwerp document. Ook is de ruimte tussen de afbeeldingen best groot.
Als ik de grootte van het window aanpas dan komen de afbeeldingen soms over elkaar heen te staan.

apps/directory/src/views/Landingpage.vue Outdated Show resolved Hide resolved
connoratrug and others added 3 commits December 16, 2024 12:48
… of github.com:molgenis/molgenis-emx2 into feat/3769-directory-update-frontpage-to-use-categories
Copy link
Member

@esthervanenckevort esthervanenckevort left a comment

Choose a reason for hiding this comment

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

_Functioneel werkt het, maar visueel is het nog behoorlijk dominant en zwaar.

  • Ik denk dat het mooier is als er minder nadrukkelijk een rand om de afbeeldingen zijn en er wat meer afbeeldingen op een regel zodat de afstand tussen twee afbeeldingen kleiner is.
  • Ook de tekst erboven mag wat minder nadruk hebben, een h3 ipv h1 vind ik zelf al mooier.
  • BBMRI-ERIC is volledig in hoofdletters
  • De tekst "Search the BBMRI-ERIC Directory by biobank, samples and collections" is incorrect, dat zou moeten zijn "Search the BBMRI-ERIC Directory by category"

Ik heb even wat met de web inspector gespeeld om de CSS on the fly aan te passen, dit is nog niet helemaal goed, maar geeft het idee wel weer:
Screenshot 2024-12-16 at 15 38 41

Hiervoor moest ik met name de percentages in de flex en max-width aanpassen zodat er meer op een regel komt. Daarnaast de border en shadow aangepast. Dat is natuurlijk bestaande CSS classes aanpassen ipv de juiste alternatieve CSS classes kiezen, dus is niet 1 op 1 over te nemen._

ik denk niet dat dit beter is

@connoratrug connoratrug dismissed esthervanenckevort’s stale review December 16, 2024 15:04

suggested changes are no an improvement

Copy link
Contributor

@chinook25 chinook25 left a comment

Choose a reason for hiding this comment

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

Found some issues:

  • After clicking on one of the filters, the history gets messed up and I need to press back 3 times to get back to the home screen.
  • When going back, the query stays in the url, it should be removed when on the home screen.
  • Clicking on 'see more detail' for a collection results in a white screen.
  • Clicking the 'go to biobank' button results in a white screen.
  • There are 4 collections previewed, but there are only 3 in the tables. Should this be dynamic?

@connoratrug
Copy link
Contributor Author

connoratrug commented Dec 18, 2024

Found some issues:

  • After clicking on one of the filters, the history gets messed up and I need to press back 3 times to get back to the home screen.

is this related to this pr ? , do you have any suggestion on how to solve this ( i checked , imo this is related to the way the filters are loaded and the bookmark and route keeps getting updated, we should fix this ( not a small fix ) but not in this pr imo, this pr does make this issue more prominent, so maybe we should fix it before adding this feature)

see: #4572

  • When going back, the query stays in the url, it should be removed when on the home screen.

is this related to this pr ? , do you have any suggestion on how to solve this ( i checked , imo this is related to the way the filters are loaded and the bookmark and route keeps getting updated, we should fix this ( not a small fix ) but not in this pr imo, this pr does make this issue more prominent, so maybe we should fix it before adding this feature)

see: #4572

  • Clicking on 'see more detail' for a collection results in a white screen.

can not reproduce

  • There are 4 collections previewed, but there are only 3 in the tables. Should this be dynamic?

not sure what you mean by this , and how it relates to this pr

…3769-directory-update-frontpage-to-use-categories
@connoratrug connoratrug merged commit 0c483ef into master Dec 18, 2024
6 of 7 checks passed
@connoratrug connoratrug deleted the feat/3769-directory-update-frontpage-to-use-categories branch December 18, 2024 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants