-
Notifications
You must be signed in to change notification settings - Fork 793
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
Add warning banner for "insufficient resources" #13146
Add warning banner for "insufficient resources" #13146
Conversation
Build Artifacts
|
215ddd8
to
a108fe1
Compare
a108fe1
to
13fe683
Compare
@radinamatic @pcenov can this go on the QA team to-do list? 🙏 |
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.
Hi @AllanOXDi - overall this is looking very good, and the changes I'm requesting are small tweaks:
- update your "context" in the string message object
- do a little bit of troubleshooting on the UiAlert dismissibility and report back - we can evaluate how important it is alongside where the bug might be coming from
...views/quizzes/CreateExamPage/sidePanels/QuizResourceSelection/subPages/QuestionsSettings.vue
Show resolved
Hide resolved
insufficientResources: { | ||
message: | ||
'There are currently only {count, number} questions across all practice resources in your library. To create a larger quiz, consider adding more resources to your library.', | ||
context: 'Message to indicate that the license is not sufficient for the user.', |
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 this context might be copied from the wrong place - can you update it?
...views/quizzes/CreateExamPage/sidePanels/QuizResourceSelection/subPages/QuestionsSettings.vue
Show resolved
Hide resolved
Hi @AllanOXDi, the following part of #13109 is not implemented: If there is a limit in the number of available questions, we should limit the number of questions that a user can add to the number of available resources. For example, in the figma spec below, the validation on the input field should have a max value of 18 (rather than 25, the section max). |
}, | ||
data() { | ||
return { | ||
inputMaxQuestions: 18, |
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 18 is coming from the spec in the issue, but that was just an example - this should be a dynamic value that is based on the number of available questions. So this should either be 25 (the max per section) or the addableQuestionCount
- I don't think you need to have a new variable here.
@@ -36,7 +43,7 @@ | |||
<KIconButton | |||
icon="plus" | |||
aria-hidden="true" | |||
:disabled="questionCount >= maxQuestions || !questionCountIsEditable" | |||
:disabled="questionCount >= inputMaxQuestions || !questionCountIsEditable" |
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 see in the diff that we were previously using maxQuestions
. Since it doesn't seem like inputMaxQuestions
is being updated dynamically, let's continue to use that, rather than add another variable here.
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.
Thank you @AllanOXDi - looks good
Summary
This update introduces a banner to alert coaches when they do not have enough available resources to create a quiz
References
#13109
Reviewer guidance