From 90611c091ce2914afc68bd36c5237c5564b1692e Mon Sep 17 00:00:00 2001 From: Philipp Imhof <52650214+PhilippImhof@users.noreply.github.com> Date: Mon, 11 Nov 2024 15:24:02 +0100 Subject: [PATCH] fix: page format letter not working properly (#38) At the same time, the fix improves how settings are stored as user preferences. --- essaydownload_form.php | 8 +++---- essaydownload_options.php | 28 +++++++++++++++---------- report.php | 2 +- tests/behat/form.feature | 2 +- tests/report_test.php | 44 +++++++++++++++++++-------------------- 5 files changed, 44 insertions(+), 40 deletions(-) diff --git a/essaydownload_form.php b/essaydownload_form.php index 0ea1174..a431ef6 100644 --- a/essaydownload_form.php +++ b/essaydownload_form.php @@ -168,13 +168,13 @@ protected function standard_preference_fields(MoodleQuickForm $mform) { * @return void */ protected function pdf_layout_fields(MoodleQuickForm $mform) { - $mform->addElement('select', 'page', get_string('page', 'quiz_essaydownload'), [ + $mform->addElement('select', 'pageformat', get_string('page', 'quiz_essaydownload'), [ 'a4' => get_string('pagea4', 'quiz_essaydownload'), 'letter' => get_string('pageletter', 'quiz_essaydownload'), ]); - $mform->setType('page', PARAM_ALPHA); - $mform->setDefault('page', 'a4'); - $mform->disabledIf('page', 'fileformat', 'neq', 'pdf'); + $mform->setType('pageformat', PARAM_ALPHANUM); + $mform->setDefault('pageformat', 'a4'); + $mform->disabledIf('pageformat', 'fileformat', 'neq', 'pdf'); $margingroup = []; $margingroup[] = $mform->createElement('text', 'marginleft', '', ['size' => 3]); diff --git a/essaydownload_options.php b/essaydownload_options.php index 080b032..c839f96 100644 --- a/essaydownload_options.php +++ b/essaydownload_options.php @@ -228,23 +228,29 @@ public function setup_from_user_preferences() { public function update_user_preferences() { set_user_preference('quiz_essaydownload_attachments', $this->attachments); set_user_preference('quiz_essaydownload_fileformat', $this->fileformat); - set_user_preference('quiz_essaydownload_fixremfontsize', $this->fixremfontsize); set_user_preference('quiz_essaydownload_flatarchive', $this->flatarchive); - set_user_preference('quiz_essaydownload_font', $this->font); - set_user_preference('quiz_essaydownload_fontsize', $this->fontsize); set_user_preference('quiz_essaydownload_groupby', $this->groupby); - set_user_preference('quiz_essaydownload_includefooter', $this->includefooter); set_user_preference('quiz_essaydownload_includestats', $this->includestats); - set_user_preference('quiz_essaydownload_linespacing', $this->linespacing); - set_user_preference('quiz_essaydownload_marginbottom', $this->marginbottom); - set_user_preference('quiz_essaydownload_marginleft', $this->marginleft); - set_user_preference('quiz_essaydownload_marginright', $this->marginright); - set_user_preference('quiz_essaydownload_margintop', $this->margintop); set_user_preference('quiz_essaydownload_nameordering', $this->nameordering); - set_user_preference('quiz_essaydownload_pageformat', $this->pageformat); set_user_preference('quiz_essaydownload_questiontext', $this->questiontext); set_user_preference('quiz_essaydownload_shortennames', $this->shortennames); - set_user_preference('quiz_essaydownload_source', $this->source); + + // The following settings should only be stored, if the user creates PDF files, because if they + // don't, the corresponding fields will be disabled and have no values, so the user pref would + // be removed and thus the field would not be pre-filled next time. + if ($this->fileformat === 'pdf') { + set_user_preference('quiz_essaydownload_fixremfontsize', $this->fixremfontsize); + set_user_preference('quiz_essaydownload_font', $this->font); + set_user_preference('quiz_essaydownload_fontsize', $this->fontsize); + set_user_preference('quiz_essaydownload_includefooter', $this->includefooter); + set_user_preference('quiz_essaydownload_linespacing', $this->linespacing); + set_user_preference('quiz_essaydownload_marginbottom', $this->marginbottom); + set_user_preference('quiz_essaydownload_marginleft', $this->marginleft); + set_user_preference('quiz_essaydownload_marginright', $this->marginright); + set_user_preference('quiz_essaydownload_margintop', $this->margintop); + set_user_preference('quiz_essaydownload_pageformat', $this->pageformat); + set_user_preference('quiz_essaydownload_source', $this->source); + } } /** diff --git a/report.php b/report.php index 6832449..e363f64 100644 --- a/report.php +++ b/report.php @@ -555,7 +555,7 @@ protected function generate_pdf(string $text, string $header = '', string $subhe $text = $this->workaround_atto_font_size_issue($text); } - $doc = new customTCPDF('P', 'mm', $this->options->pageformat); + $doc = new customTCPDF('P', 'mm', strtoupper($this->options->pageformat)); $doc->SetCreator('quiz_essaydownload plugin for Moodle LMS'); $doc->SetAuthor($author); diff --git a/tests/behat/form.feature b/tests/behat/form.feature index 6a2f7ab..a908991 100644 --- a/tests/behat/form.feature +++ b/tests/behat/form.feature @@ -54,7 +54,7 @@ Feature: Validation and display of the form When I set the field "fileformat" to "txt" Then the "source" "select" should be disabled And the "fixremfontsize" "field" should be disabled - And the "page" "select" should be disabled + And the "pageformat" "select" should be disabled And the "marginleft" "field" should be disabled And the "marginright" "field" should be disabled And the "margintop" "field" should be disabled diff --git a/tests/report_test.php b/tests/report_test.php index bf8e724..c8fb30c 100644 --- a/tests/report_test.php +++ b/tests/report_test.php @@ -1024,50 +1024,48 @@ public function test_txt_when_input_is_html(): void { * Provide data to test the Atto workaround. First line is the expected output, * second line is the input. * - * @return array + * @return \Generator */ - public static function provide_texts_with_rem_font_span(): array { - return [ - 'nothing' => [ + public static function provide_texts_with_rem_font_span(): \Generator { + yield 'nothing' => [ 'foo bar', 'foo bar', - ], - 'one span not font-size' => [ + ]; + yield 'one span not font-size' => [ 'foo bli bar', 'foo bli bar', - ], - 'one span with font-size not rem' => [ + ]; + yield 'one span with font-size not rem' => [ 'foo bli bar', 'foo bli bar', - ], - 'one span with font-size rem' => [ + ]; + yield 'one span with font-size rem' => [ 'foo bli bar', 'foo bli bar', - ], - 'one span with font-size rem and other attributes' => [ + ]; + yield 'one span with font-size rem and other attributes' => [ 'foo bli bar', 'foo bli bar', - ], - 'one span with font-size rem and other properties' => [ + ]; + yield 'one span with font-size rem and other properties' => [ 'foo bli bar', 'foo bli bar', - ], - 'one span with font-size rem, uppercase' => [ + ]; + yield 'one span with font-size rem, uppercase' => [ 'foo bli bar', 'foo bli bar', - ], - 'one span with font-size rem, single quote' => [ + ]; + yield 'one span with font-size rem, single quote' => [ "foo bli bar", "foo bli bar", - ], - 'two spans with font-size rem' => [ + ]; + yield 'two spans with font-size rem' => [ 'foo bli goo dip bar', 'foo bli goo dip bar', - ], - 'one span font-size rem with whitespace' => [ + ]; + yield 'one span font-size rem with whitespace' => [ 'foo bli bar', 'foo bli bar', - ], ]; }