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(account): restyle sign in page #1687

Open
wants to merge 54 commits into
base: main
Choose a base branch
from
Open

Conversation

dargmuesli
Copy link
Member

📚 Description

Started working on aligning the app implementation to our new design.

📝 Checklist

  • All commits follow the Conventional Commit format
  • The PR's title follows the Conventional Commit format

@dargmuesli
Copy link
Member Author

@huzaifaedhi22 I'll fully review this next week, I've burnt through a bit of my backlog.

@dargmuesli dargmuesli force-pushed the master branch 2 times, most recently from db8592b to 95ec293 Compare February 20, 2025 05:59
to?: RouteLocationRaw
type?: 'button' | 'reset' | 'submit'
}

Copy link
Member Author

Choose a reason for hiding this comment

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

empty line to be removed

Comment on lines +7 to +11

<AppLink :to="localePath('index')" :is-colored="true">
{{ t('imprint') }}
</AppLink>

Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
<AppLink :to="localePath('index')" :is-colored="true">
{{ t('imprint') }}
</AppLink>
<AppLink :to="localePath('index')" :is-colored="true">
{{ t('imprint') }}
</AppLink>


<i18n lang="yaml">
de:
terms: Bedingungen
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
terms: Bedingungen
terms: Nutzungsbedingungen

@@ -104,6 +145,7 @@ const rules = {
password: VALIDATION_PASSWORD(),
emailAddress: VALIDATION_EMAIL_ADDRESS({ isRequired: true }),
}

Copy link
Member Author

Choose a reason for hiding this comment

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

empty line to be removed, it's all the vuelidate part


const legalTerms = computed(() => {
return (
legalTermsQuery.value?.allLegalTerms?.edges
Copy link
Member Author

Choose a reason for hiding this comment

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

What about .nodes instead of .edges? We use nodes in all other places

</div>

Copy link
Member Author

Choose a reason for hiding this comment

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

unnecessary empty line

@@ -21,5 +21,5 @@ useHeadDefault({ title })
de:
title: Registrieren
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
title: Registrieren
title: Erstelle ein Konto

Copy link
Member Author

Choose a reason for hiding this comment

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

Master should be merged in to resolve these diffs

@huzaifaedhi22 huzaifaedhi22 force-pushed the feat/account/restyle branch from 02166c7 to 9df88a4 Compare March 6, 2025 02:55
@dargmuesli dargmuesli changed the base branch from master to main March 7, 2025 10:28
--critic-string: #e00000;
--faint-line: #f2f2f2;
--faint-weak: #f2f2f2;
--semantic-accent-accent-icon: #12683a;
--semantic-accent-accent-text: #12683a;
Copy link
Member Author

Choose a reason for hiding this comment

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

Why remove any line here?

@@ -304,6 +305,15 @@
);
}

@utility vio-line-clamp-2 {
Copy link
Member Author

Choose a reason for hiding this comment

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

Why add this again?

@@ -0,0 +1,406 @@
/* stylelint-disable at-rule-no-deprecated */
Copy link
Member Author

Choose a reason for hiding this comment

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

This file is now called app.css, maevsi.css does not exist anymore.

'rounded-sm',
...(props.isColored ? ['text-link-dark dark:text-link-bright'] : []),
'rounded',
...(props.isColored ? ['text-accent-strong dark:text-link-bright'] : []),
Copy link
Member Author

Choose a reason for hiding this comment

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

Does this work? Shouldn't it be

Suggested change
...(props.isColored ? ['text-accent-strong dark:text-link-bright'] : []),
...(props.isColored ? ['text-(--accent-strong= dark:text-link-bright'] : []),

?

Comment on lines +49 to +50
:is-underlined="true"
:is-colored="true"
Copy link
Member Author

Choose a reason for hiding this comment

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

Suggested change
:is-underlined="true"
:is-colored="true"
is-underlined
is-colored

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.

2 participants