Skip to content
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 FAQ Help #84

Merged
merged 7 commits into from
Oct 20, 2024
Merged

Add FAQ Help #84

merged 7 commits into from
Oct 20, 2024

Conversation

iMattPro
Copy link
Contributor

@iMattPro iMattPro commented Oct 18, 2024

Screenshot 2024-10-18 at 3 04 25 PM

Signed-off-by: Matt Friedman <[email protected]>
Comment on lines 289 to 320
public function wpn_faq($event)
{
if ($event['block_name'] === 'HELP_FAQ_BLOCK_ATTACHMENTS')
{
$this->language->add_lang('webpushnotifications_faq', 'phpbb/webpushnotifications');

$this->template->assign_block_vars('faq_block', [
'BLOCK_TITLE' => $this->language->lang('HELP_FAQ_WPN'),
'SWITCH_COLUMN' => false,
]);

$questions = [
'HELP_FAQ_WPN_WHAT_QUESTION' => 'HELP_FAQ_WPN_WHAT_ANSWER',
'HELP_FAQ_WPN_HOW_QUESTION' => 'HELP_FAQ_WPN_HOW_ANSWER',
'HELP_FAQ_WPN_IOS_QUESTION' => 'HELP_FAQ_WPN_IOS_ANSWER',
'HELP_FAQ_WPN_SESSION_QUESTION' => 'HELP_FAQ_WPN_SESSION_ANSWER',
'HELP_FAQ_WPN_SUBBING_QUESTION' => 'HELP_FAQ_WPN_SUBBING_ANSWER',
'HELP_FAQ_WPN_GENERAL_QUESTION' => 'HELP_FAQ_WPN_GENERAL_ANSWER',
];

$faq_rows = [];
foreach ($questions as $question => $answer)
{
$faq_rows[] = [
'FAQ_QUESTION' => $this->language->lang($question),
'FAQ_ANSWER' => $this->language->lang($answer),
];
}

$this->template->assign_block_vars_array('faq_block.faq_row', $faq_rows);
}
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I feel like we're duplicating the code from the help manager here. Instead we could just inject the manager service and use it like f.e.

$this->language->add_lang('webpushnotifications_faq', 'phpbb/webpushnotifications');
$questions = [
  'HELP_FAQ_WPN_WHAT_QUESTION'    => 'HELP_FAQ_WPN_WHAT_ANSWER',
  'HELP_FAQ_WPN_HOW_QUESTION'     => 'HELP_FAQ_WPN_HOW_ANSWER',
  'HELP_FAQ_WPN_IOS_QUESTION'     => 'HELP_FAQ_WPN_IOS_ANSWER',
  'HELP_FAQ_WPN_SESSION_QUESTION' => 'HELP_FAQ_WPN_SESSION_ANSWER',
  'HELP_FAQ_WPN_SUBBING_QUESTION' => 'HELP_FAQ_WPN_SUBBING_ANSWER',
  'HELP_FAQ_WPN_GENERAL_QUESTION' => 'HELP_FAQ_WPN_GENERAL_ANSWER',
];
$this->help_manager->add_block('HELP_FAQ_WPN', false, $questions);

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or will it make an endless loop, hmm.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah but I dont want to write the tests for that just to eliminate a few lines of code in here

Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
Signed-off-by: Matt Friedman <[email protected]>
@iMattPro
Copy link
Contributor Author

@marc1706 do you think this is worth adding to the core too?

@iMattPro iMattPro merged commit ca6e1ab into phpbb-extensions:main Oct 20, 2024
32 checks passed
@iMattPro iMattPro deleted the add-faq branch October 20, 2024 00:49
@marc1706
Copy link

@marc1706 do you think this is worth adding to the core too?

@iMattPro yeah, I think it would be nice to have this in the core as well. Can you create a PR for that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants