-
Notifications
You must be signed in to change notification settings - Fork 0
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
Cdc thematic views - Landing page for thematic views, filter/field exclusions, routing #53
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! Changes sound good and also look good from briefly checking on my dev server. Some notes:
- I guess this is the same branch that was merged and deleted once on GitHub already so it resulted in some conflicts that need to be resolved before it can be merged into main. Probably better to do the merge/rebase locally and then push. I've never tried GitHub's 'Resolve conflicts' though so maybe it would be decent.
- I think we should not hack Instantsearch widget source files but we should also not create custom widgets just to fix these small issues so instead we can just document these issues in accessibility statement.
- Scroll restoration now seems to work on Chromium based browsers but not on Firefox. On Firefox the page will be slightly scrolled but still have part of search bar visible and focused instead of fully scrolling to where it was.
@@ -59,7 +58,7 @@ | |||
"@babel/plugin-proposal-class-properties": "7.18.6", | |||
"@babel/plugin-transform-runtime": "7.21.0", | |||
"@babel/preset-env": "7.20.2", | |||
"@babel/preset-react": "7.18.6", | |||
"@babel/preset-react": "^7.26.3", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This updated package is now not locked to this specific version.
export const thematicViews: readonly ThematicView[] = [ | ||
{ | ||
"key": "cdc", | ||
"path": "/", | ||
"defaultIndex": "cmmstudy_en", | ||
"title": "Data Catalogue", | ||
"longTitle": "CESSDA Data Catalogue", | ||
"listDescription": "The CESSDA Data Catalogue master collection contains descriptions of more 40,000 data collections held by CESSDA’s Service Providers (SPs), originating from over 20 European countries.", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Small mistake in text: missing 'than' in 'more than'.
} | ||
] | ||
], | ||
excludeFields: ["publisher"], |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think publisher should still be shown on detail page, at least personally I was only talking about excluding filter.
Changes from last PR:
New functionality:
Notes and fixes:
Todos: