feat: TR-4179 new html5 elements for QTI SDK #312

kilatib · 2022-05-25T15:16:12Z

TR-4179

For tests:

Merge PR 125
Import Test from ticket
Try to publish DE in a backoffice

kilatib · 2022-05-25T15:16:45Z

src/qtism/data/content/xhtml/html5/Figcaption.php

+ * Copyright (c) 2022 (original work) Open Assessment Technologies SA;
+ */
+
+declare(strict_types=1);


not sure I could keep it but for draft I left it

wazelin · 2022-05-25T15:32:58Z

I believe you need to add figcaption to https://github.com/oat-sa/qti-sdk/blob/develop/src/qtism/data/storage/xml/marshalling/ContentMarshaller.php#L96 and figure to https://github.com/oat-sa/qti-sdk/blob/develop/src/qtism/data/storage/xml/marshalling/ContentMarshaller.php#L129

…mples

poyuki · 2022-05-26T07:45:59Z

src/qtism/data/content/xhtml/html5/Figcaption.php

+    private $content;
+
+    /**
+     * Create a new Div object.


i believe not div

poyuki · 2022-05-26T07:46:42Z

src/qtism/data/content/xhtml/html5/Figure.php

+    private $content;
+
+    /**
+     * Create a new Div object.


i believe not div

poyuki · 2022-05-26T07:58:14Z

src/qtism/data/storage/xml/marshalling/Html5FigcaptionMarshaller.php

+    protected function marshall(QtiComponent $component): DOMElement
+    {
+        /** @var Figcaption $component */


WDYT?

Suggested change

protected function marshall(QtiComponent $component): DOMElement

{

/** @var Figcaption $component */

/**

* @param QtiComponent&Figcaption $component

*/

protected function marshall(QtiComponent $component): DOMElement

{

poyuki · 2022-05-26T07:59:14Z

src/qtism/data/storage/xml/marshalling/Html5FigureMarshaller.php

+    protected function marshall(QtiComponent $component): DOMElement
+    {
+        /** @var Figure $component */


WDYT?

Suggested change

protected function marshall(QtiComponent $component): DOMElement

{

/** @var Figure $component */

/**

* @param QtiComponent&Figure $component

*/

protected function marshall(QtiComponent $component): DOMElement

{

poyuki · 2022-05-26T07:59:57Z

src/qtism/data/content/xhtml/html5/Figcaption.php

+{
+    use FlowTrait;
+
+    public const QTI_CLASS_NAME = 'figure';


does it also should be figure?

src/qtism/runtime/rendering/markup/xhtml/FigcaptionRenderer.php

src/qtism/runtime/rendering/markup/xhtml/FigureRenderer.php

src/qtism/runtime/rendering/markup/xhtml/XhtmlRenderingEngine.php

src/qtism/data/storage/xml/marshalling/Qti22MarshallerFactory.php

pnal

Please check the tests, since I was unable to run almost all of them

pnal · 2022-05-26T17:34:47Z

test/qtismtest/data/storage/xml/marshalling/Html5FigcaptionMarshallerTest.php

+        $class = 'testclass';
+
+        $expected = sprintf(
+            '<%1$s id="%2$s " class="%3$s">text content</%1$s>',


you are expecting text content whereas not adding it to the object that you serializing

Could you please point me to how to do it without recreating the recursive marshaler for html5 element?

What means without recreating?

no matter I found the answer in PR which you did

thank you for the help

pnal · 2022-05-26T17:35:26Z

test/qtismtest/data/storage/xml/marshalling/Html5FigcaptionMarshallerTest.php

+     */
+    public function testMarshall22WithDefaultValues(): void
+    {
+        $expected = sprintf('<%1$s>text content</%1$s>', $this->namespaceTag(Figcaption::QTI_CLASS_NAME));


pnal · 2022-05-26T17:37:15Z

test/qtismtest/data/storage/xml/marshalling/Html5FigcaptionMarshallerTest.php

+            $class,
+        );
+
+        $expected = new Figcaption($id, $class);


did you check that result object contains text content?

pnal · 2022-05-26T17:40:54Z

test/qtismtest/data/storage/xml/marshalling/Html5FigureMarshallerTest.php

+        $class = 'testclass';
+
+        $xml = sprintf(
+            '<%1$s id="%2$s " class="%3$s"></%1$s>',


what about children elements, is it able to unmarshall them?

pnal · 2022-05-26T17:44:56Z

src/qtism/data/content/xhtml/html5/Figcaption.php

+     */
+    public function __construct($id = '', $class = '', $lang = '', $label = '')
+    {
+        parent::__construct($id, $class, $lang, $label);


the parent (Html5Element) constructor:

public function __construct( $title = null, $role = null, $id = null, $class = null, $lang = null, $label = null )

pnal · 2022-05-26T17:45:12Z

src/qtism/data/content/xhtml/html5/Figure.php

+     */
+    public function __construct($id = '', $class = '', $lang = '', $label = '')
+    {
+        parent::__construct($id, $class, $lang, $label);


the parent (Html5Element) constructor:

public function __construct( $title = null, $role = null, $id = null, $class = null, $lang = null, $label = null )

pnal · 2022-05-26T17:47:44Z

test/qtismtest/data/content/xhtml/html5/FigcaptionTest.php

+
+        $subject = new Figcaption($id, $class);
+
+        self::assertEquals($id, $subject->getId());


this will fail constantly cause you can not set $id and $class because of:
https://github.com/oat-sa/qti-sdk/pull/312/files#diff-cb9fba2d3b91cd5fb8765ade051c0fc88be45eef69edb1ae6b1f537aca9e9c65R54

pnal · 2022-05-26T17:49:59Z

test/qtismtest/data/content/xhtml/html5/FigureTest.php

+
+        $subject = new Figure($id, $class);
+
+        self::assertEquals($id, $subject->getId());


same issue as for figcaption similar test

pnal · 2022-06-01T13:24:35Z

src/qtism/data/storage/xml/marshalling/trait/QtiHtml5AttributeTrait.php

+
+declare(strict_types=1);
+
+namespace qtism\data\storage\xml\marshalling\trait;


keyword trait as a part of a namespace:
PHP Fatal error: Uncaught ParseError: syntax error, unexpected 'trait' (T_TRAIT), expecting identifier (T_STRING) or '{'

wazelin

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
Pull request's target is not master

feat: TR-4179 new html5 elements for QTI SDK

0bc1e86

kilatib commented May 25, 2022

View reviewed changes

kilatib added 2 commits May 25, 2022 17:18

fix: add empty lines to end of files

8e9a2a8

tests: add empty lines to end of files

0344146

kilatib added 2 commits May 25, 2022 17:37

feat: TR-4179 added renderer classes

7ad3816

fix: use constant where it possible; change component placement to si…

ffa1fb7

…mples

poyuki requested changes May 26, 2022

View reviewed changes

pnal requested changes May 26, 2022

View reviewed changes

kilatib added 4 commits May 26, 2022 23:36

fix: small partly fix attribute orders

6b8e6ab

tests: small partly fix for attribute order

98fc4dc

fix: parse recrusive new tag for add content

d28ba91

fix: use proper class to build qti elements

a2c5506

kilatib requested review from poyuki and wazelin May 31, 2022 07:23

kilatib changed the title ~~Draft: feat: TR-4179 new html5 elements for QTI SDK~~ feat: TR-4179 new html5 elements for QTI SDK May 31, 2022

kilatib added 2 commits May 31, 2022 13:46

tests: Simple marchalling and unmarchalling for new html5 elements

55df16f

fix: correct marchalling for: figre and figcaption elements

ddfc46c

kilatib requested a review from pnal May 31, 2022 11:47

fix: remove https as it was before

4fa5828

pnal reviewed Jun 1, 2022

View reviewed changes

fix: TR-4179 remove namespace because it not support in php 7.4

2c0a345

kilatib requested a review from pnal June 1, 2022 14:02

wazelin approved these changes Jun 3, 2022

View reviewed changes

wazelin merged commit 8a06eb4 into develop Jun 3, 2022

wazelin deleted the feature/TR-4179/support-qti-figure-elm branch June 3, 2022 07:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: TR-4179 new html5 elements for QTI SDK #312

feat: TR-4179 new html5 elements for QTI SDK #312

kilatib commented May 25, 2022 •

edited by poyuki

Loading

kilatib May 25, 2022

wazelin May 25, 2022

wazelin commented May 25, 2022

poyuki May 26, 2022

poyuki May 26, 2022

poyuki May 26, 2022 •

edited

Loading

poyuki May 26, 2022 •

edited

Loading

poyuki May 26, 2022

pnal left a comment

pnal May 26, 2022

kilatib May 26, 2022

pnal May 30, 2022

kilatib May 30, 2022

kilatib May 30, 2022

pnal May 26, 2022

pnal May 26, 2022

pnal May 26, 2022

pnal May 26, 2022

pnal May 26, 2022

pnal May 26, 2022

pnal May 26, 2022

pnal Jun 1, 2022

wazelin left a comment


		$subject = new Figcaption($id, $class);

		self::assertEquals($id, $subject->getId());


		$subject = new Figure($id, $class);

		self::assertEquals($id, $subject->getId());


		declare(strict_types=1);

		namespace qtism\data\storage\xml\marshalling\trait;

feat: TR-4179 new html5 elements for QTI SDK #312

feat: TR-4179 new html5 elements for QTI SDK #312

Conversation

kilatib commented May 25, 2022 • edited by poyuki Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wazelin commented May 25, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

poyuki May 26, 2022 • edited Loading

Choose a reason for hiding this comment

poyuki May 26, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pnal left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

wazelin left a comment

Choose a reason for hiding this comment

kilatib commented May 25, 2022 •

edited by poyuki

Loading

poyuki May 26, 2022 •

edited

Loading

poyuki May 26, 2022 •

edited

Loading