-
Notifications
You must be signed in to change notification settings - Fork 11
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
Feature/ADF-1695/Show multiple preview buttons #2485
Conversation
Front-end summary Node 18
|
/** | ||
* We assume the ClientLibConfigRegistry is filled up with something like this: | ||
* 'taoQtiTest/controller/creator/creator' => [ | ||
* 'provider' => 'qtiTest', | ||
* ], | ||
* | ||
* Or, with something like this for allowing multiple buttons in case of several providers are available: | ||
* 'taoQtiTest/controller/creator/creator' => [ | ||
* 'provider' => 'qtiTest', | ||
* 'providers' => [ | ||
* ['id' => 'qtiTest', 'label' => 'Preview'], | ||
* ['id' => 'xxxx', 'label' => 'xxxx'], | ||
* ... | ||
* ], | ||
* ], | ||
*/ |
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've mentioned it in the other PR about items. This should be in a private method getProviders
JSDoc describing the contract and helping even IDE autocomplete and documenting the code (since we do not have domain objects for this).
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.
LGTM. Code review only :)
Thanks @jsconan
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
Version
There are 0 BREAKING CHANGE, 3 features, 0 fix |
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.
Looks good to me, nice job!
- New code is covered by tests (if applicable)
- Tests are running successfully (old and new ones) on my local machine (if applicable)
- New code is respecting code style rules
- New code is respecting best practices
- New code is not subject to concurrency issues (if applicable)
- Feature is working correctly on my local machine (if applicable)
- Acceptance criteria are respected
- Pull request title and description are meaningful
Related to: https://oat-sa.atlassian.net/browse/ADF-1695
Requires:
Summary
Add the possibility of having more than one test previewer.
Details
It supports multiple preview buttons from the test's property page.
This is made thanks to a configuration applied in
config/tao/client_lib_config_registry.conf.php
, and a change applied to thestructures.xml
file.Each button must be declared with an identifier starting with
test-preview
. The first button must betest-preview
, the secondtest-preview-1
, etc. The suffix number refers to a position in the configuration (seeclient_lib_config_registry.conf.php
).Example:
The configuration can be extended with a list of possible providers, aside from the default one. This is done in
config/tao/client_lib_config_registry.conf.php
.Example:
How to test
git checkout -t origin/feature/ADF-1695/multiple-preview-buttons