-
Notifications
You must be signed in to change notification settings - Fork 1
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
Changing the card gallery to one card on the main page #145
base: main
Are you sure you want to change the base?
Conversation
Majapur1
commented
Nov 6, 2024
- Added one card to the home screen instead of the gallery (it stayed within the gallery, it is possible to add more cards if needed).
Quality Gate passedIssues Measures |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #145 +/- ##
==========================================
+ Coverage 26.83% 27.11% +0.28%
==========================================
Files 93 93
Lines 3455 3419 -36
Branches 122 122
==========================================
Hits 927 927
+ Misses 2461 2425 -36
Partials 67 67 ☔ View full report in Codecov by Sentry. |
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 - made a couple of suggestions
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 png file is huge! try converting it to a jpg :-) also I don't see it being used anywhere?
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.
second what @lkeegan said. Do we need it at all?
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.
Ok! I've added it for use in the next element. I'll compress it and add it again later
@@ -27,36 +27,34 @@ export let styleProps = { | |||
imgClass="max-md:hidden object-scale-down" | |||
href={data.button ? null : data.href} | |||
class={data.button | |||
? 'm-2 max-w-prose items-center text-gray-700 dark:text-white' | |||
? 'm-2 max-w-prose min-w-full items-center bg-[#a4d2d0] text-gray-700 dark:text-white' |
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 the CardDisplay and GalleryDisplay components are used in multiple places, not just on the front page (@MaHaWo?).
It might make more sense for you to directly use a single <Card>
element on the frontpage which you can customize however you like, rather than modifying these components?
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.
yes, they are indeed, I would also suggest to use Card
directly if you need only one of them. also, I would suggest using colors that are defined centrally in tailwind-config.ts
, because a unified style will depend on the color hexcode being copied everywhere we need it, which is brittle
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 your help and suggestions! I'll use the Card element in those cases and add other colors to tailwind-config.ts
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.
a few remarks here and there.
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.
second what @lkeegan said. Do we need it at all?
@@ -27,36 +27,34 @@ export let styleProps = { | |||
imgClass="max-md:hidden object-scale-down" | |||
href={data.button ? null : data.href} | |||
class={data.button | |||
? 'm-2 max-w-prose items-center text-gray-700 dark:text-white' | |||
? 'm-2 max-w-prose min-w-full items-center bg-[#a4d2d0] text-gray-700 dark:text-white' |
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.
yes, they are indeed, I would also suggest to use Card
directly if you need only one of them. also, I would suggest using colors that are defined centrally in tailwind-config.ts
, because a unified style will depend on the color hexcode being copied everywhere we need it, which is brittle
@@ -108,7 +108,7 @@ $: filteredComponentProps = filteredItems.map((item) => { | |||
{/if} | |||
|
|||
<Gallery | |||
class="grid w-full grid-cols-1 justify-center gap-8 p-4 sm:grid-cols-1 md:grid-cols-2 lg:grid-cols-3 xl:grid-cols-4" | |||
class="w-full justify-items-center" |
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.
same here. While the styling isn´t final, some of the other components that use the GalleryDisplay
depend on it being styled as originally defined and their styling will break if it is changed.
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.
Ok, get it.
header: "Was ist Mondey?", | ||
summary: | ||
"Mondey ist ein wissentschaftlich geprüftes Programm zure Dokumentation der Entwicklung von Kindern bis 6 Jahren.", | ||
header: "Möchten Sie die Entwicklung von Kindern begleiten und fördern?", |
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.
you can use the svelte-i18n
package to make use of localization. There is currently one default localization file under frontend/src/locales
which is called de.json
. You can define a section "frontpage" there and add all the items you need:
"frontpage": {
"heading": "Möchten Sie die Entwicklung von Kindern begleiten und fördern?",
"summary": "Hier sind Sie genau richtig!",
"buttonLabel": "Anmeldung"
}
which then can be used in the *.svelte
files like so:
import {_} from "svelte-i18n";
export let items = [
{header: $_("frontpage.heading"), ... }
and so on. If you want to use these in the markup section of the component, you have to wrap it in {}
.
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.
Okay, I'll do it.
Thank you!