-
-
Notifications
You must be signed in to change notification settings - Fork 811
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: Add summary to recipe instructions #4410
feat: Add summary to recipe instructions #4410
Conversation
@@ -339,7 +374,7 @@ export default defineComponent({ | |||
// =============================================================== | |||
// UI State Helpers | |||
|
|||
function validateTitle(title: string | undefined) { | |||
function hasSectionTitle(title: string | undefined) { |
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 has no direct relevance to the PR, but I renamed this function to be more accurate to what it's doing.
I think there is some potential in simplifiying the UI here. |
I like that idea @Kuchenpirat . Do you have a few lines suggestion of a code change to achieve that? |
<span :class="isEditForm ? 'handle' : ''"> | ||
<v-icon v-if="isEditForm" size="26" class="pb-1">{{ $globals.icons.arrowUpDown }}</v-icon> | ||
{{ $t("recipe.step-index", { step: index + 1 }) }} | ||
{{ step.summary ? step.summary : $t("recipe.step-index", { step: index + 1 }) }} | ||
</span> | ||
<template v-if="isEditForm"> |
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 should allow the user to directly edit the step summary.
Using placeholder colors the default "Step #" grey communicating that this can be changed which makes the feature more easily discoverable.
<span :class="isEditForm ? 'handle' : ''"> | |
<v-icon v-if="isEditForm" size="26" class="pb-1">{{ $globals.icons.arrowUpDown }}</v-icon> | |
{{ $t("recipe.step-index", { step: index + 1 }) }} | |
{{ step.summary ? step.summary : $t("recipe.step-index", { step: index + 1 }) }} | |
</span> | |
<template v-if="isEditForm"> | |
<v-text-field | |
v-if="isEditForm" | |
v-model="step.summary" | |
class="headline handle" | |
hide-details | |
dense | |
solo | |
flat | |
:placeholder="$t('recipe.step-index', { step: index + 1 })" | |
> | |
<template #prepend> | |
<v-icon size="26">{{ $globals.icons.arrowUpDown }}</v-icon> | |
</template> | |
</v-text-field> | |
<span v-else> | |
{{ step.summary ? step.summary : $t("recipe.step-index", { step: index + 1 }) }} | |
</span> |
Thanks for the code suggestion @Kuchenpirat, I love it! The UX is a lot better now, and the code is more concise too. |
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.
Happy to hear, seeing the amount of code the change removed i would say sometimes the easier solution is the better solution 👍
LGTM
What type of PR is this?
What this PR does / why we need it:
Introduce a "summary" for recipe instructions, which allows you to give an instruction a "name" beyond the generic "Step 1", "Step 2", etc...
You can set the summary blank to clear it and revert to the default.
The summary shows for "completed" or non-completed steps. It operates independently of section titles.
Which issue(s) this PR fixes:
None that I know of. Commonly requested feature on Discord at least.
Special notes for your reviewer:
I put it at the top of the list as I expect it might become the most commonly used option.
I'm not super wedded to the wording I've used, so if you have suggestions for better wording please let me know.
Testing
Manual through the UI.