Replace questions sidepanel #11345
Replace questions sidepanel #11345
Conversation
github-actions
bot
added
APP: Coach
Build Artifacts
|
AllanOXDi
requested review from
nucleogenesis,
MisRob,
marcellamaki and
thanksameeelian
Does this mean we should wait for review here until feedback in the aforementioned PR is addressed and it's merged? |
Yes! I think that would be ideal . |
| margin-top: 1em; | ||
| } | ||
| .choose-answer-style{ | ||
| background-color:#FAFAFA; |
There was a problem hiding this comment.
We don't hardcode colors into Kolibri (except a very few cases due to technical limitations). Tokens are used instead. This applies to other places with colors in this PR. Please see https://design-system.learningequality.org/colors/
| :layout12="{ span: 1 }" | ||
| :layout8="{ span: 1 }" | ||
| > | ||
| <div :class="isSelected ? 'circular-border-active' : 'circular-border'"> |
There was a problem hiding this comment.
How will we indicate that something is selected to screen readers?
| > | ||
| <div class="bottom-bar-style"> | ||
| <KButton | ||
| text="replace" |
There was a problem hiding this comment.
Seems like a missing translation string?
| }, | ||
| $trs: { | ||
| replaceQuestionText: { | ||
| message: 'Replace questions', |
There was a problem hiding this comment.
I thought we will be using the common translations approach for all new features, is there consensus about it? @marcellamaki
It's described here in more detail https://kolibri-dev.readthedocs.io/en/develop/i18n.html#common-string-modules
|
Thanks @AllanOXDi, lots of work here! Due to the upcoming absences, I won't be able to do a complete review in a reasonable timeframe, apologies. If it's still valid the week after next, let me know and I can jump back in. At least, I'm leaving a few notes from a very brief skim through. |
|
Thanks @MisRob. @AllanOXDi - I think some of Misha's point have already been addressed by your work in #11314 and rebasing here may resolve some of that feedback. Let me know if you want some help with that. Otherwise, let me know when you have updated, and I'll be happy to do another review since Misha will be out for some days next week. 🙂 |
github-actions
bot
added
DEV: dev-ops
github-actions
bot
removed
DEV: backend
Summary
This PR was separated from #11314 to simplify testing and speedup the development process.
Note : This should not be merged before #11314
References
#11016
Reviewer guidance
To ensure easy testing of this future, please test #11314 first then come back to this.
While following this figma design
Testing checklist
PR process
Reviewer checklist
yarnandpip)