As per the Drupal 8 release schedule , this is right time to start porting this great module.

CommentFileSizeAuthor
#1 advanced_help-1928218.patch34.91 KBmanu4543
Members fund testing for the Drupal project. Drupal Association Learn more

Comments

manu4543’s picture

Status: Active » Needs review
FileSize
34.91 KB

submitting patch

Manuel Garcia’s picture

Status: Needs review » Needs work
+++ b/advanced_help.module
@@ -501,6 +416,20 @@ function advanced_help_topic_page($module, $topic) {
+/**
+ * Implements hook_init().
+ *
+ * We need to exclude overlay js files for all page request in popup
+ */
+function advanced_help_init() {
+  $request = drupal_container()->get('request');
+  $popup = $request->query->get('popup');
+  $popup = isset($popup) && user_access('view advanced help popup');	if($popup) {
+	  overlay_set_mode('none');
+	}
+}
+

hook_init has been removed, see:
https://drupal.org/node/2013014

Manuel Garcia’s picture

Issue summary: View changes

complemented text

cweagans’s picture

Issue summary: View changes

Not only is hook_init gone, but so is overlay, so you can just remove that whole function without causing any problems :)

gilmord’s picture

Hi,
I`d like to help you with porting oh this module to Drupal 8.
Is it still actual?

gisle’s picture

Assigned: monika3517 » Unassigned

@gilmord (and others interested in creating a version of ADvanced Help for Drupal 8:

I've just signed on as maintainer for this module, and I would love to see someone working on a Drupal 8 version.

Unless you have a CV that shows some experience with Drupal, I would like to see some code before assigning co-maintainership with responsibility to oversee development for Drupal 8. The best way is probably to create a sandbox with an initial Drupal 8 version (i.e. not only a stub). I'll review the sandbox and if looks OK, I'll assign co-maintainership.

gisle’s picture

Version: 7.x-1.x-dev » 8.x-1.x-dev

Setting correct version.

gnuget’s picture

I want to take a stab at this, I will create a sandbox and let you know when I have something usable.

gisle’s picture

@gnuget,
great! Let me know when you have something reviewable.

gnuget’s picture

Hi @gisle.

You can check my progress here:

https://www.drupal.org/sandbox/gnuget/2488438

So far the module can be installed and it has a basic functionality.

There are still a ton of things to do a few of them are:

  • Using YML files instead of the ini files (well, In my version I use arrays because it was more easy to implement/parse and I wasn't sure to spent time parsing the ini files if we can use YAML files, I can use YAML or the old INI files I just want your feedback first.
  • Implement the code to make translatable the documentation.
  • Create the database table
  • Create a icon for the menu.
  • Some titles are missing

I will continue working on this, and I will be commenting here my progress.

  • gisle committed 02491c0 on 8.x-1.x authored by gnuget
    Issue #1928218 by gnuget: Initial version of upgrade to Drupal 8
    
gisle’s picture

Assigned: Unassigned » gnuget
Category: Feature request » Task

@gnuget,
great job!

I've added the code in your sandbox to the repo and made you a co-maintainer of the project. You should now be able to push to the regular Advanced help repo.

I can use YAML or the old INI files I just want your feedback first.

I think that to keep to the "spirit" of Drupal 8, we should use YAML within Advanced help as well.

gnuget’s picture

Great, I will push my changes in the regular repo now.

Thanks.

Manuel Garcia’s picture

This is great news muchas gracias @gnuget!

gisle’s picture

Create a icon for the menu.

@gnuget, what do you think of this one? #2492565: Why clutter the admin menu? Move Advanced Help under Help

I am travelling just now, so I won't have time to take a proper look at it until later, but I am leaning towards positive. If we move it, no separate icon for Advanced help will be needed.

gnuget’s picture

Sadly even if we move it, we will need a separate icon, because the default menu can't have submenus. :(

But the idea is good because Administration menu is very popular, and a lot of users use it.

gnuget’s picture

Now the module is functional, it works and the only change is we need to use the YML files instead of the INI files.

I just wanted give you an update of the status of this because the next week I will be traveling so I will not have much time to work on this until I come back (I come back in June 8).

The are still some things missing:

  • The integration with the markdown module is not done
  • We need tests.
  • I haven't tested with a different language but it should works.
  • The search integration
  • Port the patch of the issue 2492565 to this branch.
  • There is still not possible to use a custom CSS for the doc pages.
  • Cache.

If someone of you want to work in something specific of the things above, feel free to create an issue.

Regards.

mbrett5062’s picture

Just for information, you can do the following in the "AdvancedHelpController"

Replace all instances of l() and t() with the following:

$this->l() & $this->t()

See this article for an explanation why this is good.

A Peek at Traits in Drupal 8

Also I have locally changed the following section of code around handling "Markdown" files.

      // @todo check the status of the markdown filter module for D8.
      if (isset($info['readme file']) && $info['readme file']) {
        $ext = pathinfo($file, PATHINFO_EXTENSION);
        $url = new Url('https://www.drupal.org/project/markdown');
        $readme = '';
        if ('md' == $ext) {
          $readme .=
            '<p>' .
            t('If you install the !module module, the text below will be filtered by the module, producing rich text.',
              array(
                '!module' => $this->l($this->t('Markdown filter'),
                  $url,
                  array('attributes' => array('title' => $this->t('Link to project.'))))
              )) . '</p>';
        }

        $readme .=
          '<div class="advanced-help-topic"><pre class="readme">' . SafeMarkup::checkPlain($output) . '</pre></div>';
        $output = $readme;
        return $output;
      }

Still not working for .md files, but I think this is an improvement. Issue may be with Markdown module or Libraries module.

gnuget’s picture

Hi @mbrett5062

Thanks for the link related with the t() and l() functions, I've changed the code accordingly in my last commit.

Also I added support for the readme.md files.

I will continue working on this.

Regards.

gnuget’s picture

Status: Needs work » Fixed

I'm going to close this issue because the branch now exists and we can report further issues directly in the issue queue.

Thanks!

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.