Support from Acquia helps fund testing for Drupal Acquia logo

Comments

agentrickard’s picture

Status: Active » Needs work
FileSize
880 bytes

Still failing. This patch cleans things up some, but there is a fatal JS error that prevents the tab labels from rendering.

Even swapping the generated form out with working core form code failed. Only the first item appears, and the links fail entirely.

JS error (from Firebug):

Drupal.theme.prototype[func] is undefined
[Break On This Error] 	

return (Drupal.theme[func] || Drupal.theme.prototype[func]).apply(this, args);

I don't know enough about Drupal's JS -- this function is 3 lines long -- to debug further.

xumepadismal’s picture

Have the same issue. It seems that problem is the "Maintenance page" style example. theme('maintenance_page') inserts entire HTML with <script>s and <link>s to page and breaks all layout. Don't sure the best solution for this. Maybe iframe?

Anyway, I've commented lines 216-220 in 'styleguide.styleguide.inc' for now to have possibility to test verticals tabs and it works for me.

agentrickard’s picture

Nice sleuthing!

dozymoe’s picture

Here's the patch.

Can't do maintenance page preview per installed themes, would only display the default theme. It has something todo with the location of the file maintenance-page.tpl.php, which is stored in the theme registry.

Hacking around, I tried:

global $conf, $theme;
$conf['maintenance_theme'] = $my_theme;
$theme = NULL;

require_once DRUPAL_ROOT . '/includes/theme.maintenance.inc';
_drupal_maintenance_theme();

drupal_static_reset('theme_get_registry');

But the preview page also listed the default theme css files. drupal_static_reset('drupal_add_css'); just removed all of css added by modules and themes :P

edit: er, that code was in the page callback.

dozymoe’s picture

Status: Needs work » Needs review

Eh, forgot the set the flag.

agentrickard’s picture

You have to use l() and url() or else it breaks when running in a subdirectory.

dead_arm’s picture

Status: Needs review » Needs work
+++ b/styleguide.moduleundefined
@@ -27,10 +27,28 @@ function styleguide_menu() {
+  // The maintenance page preview (its theme function returns full html
+  // document with doctype :( )

Clean up this comment (no emoticons, proper punctuation, capitalize HTML)

+++ b/styleguide.moduleundefined
@@ -27,10 +27,28 @@ function styleguide_menu() {
+    'page callback' => '_styleguide_maintenance_page',
...
+function _styleguide_maintenance_page() {

Page callback functions shouldn't start with underscores.

+++ b/styleguide.styleguide.incundefined
@@ -215,7 +215,7 @@ function styleguide_styleguide() {
+    'content' => l(t('See full page'), 'admin/appearance/styleguide/maintenance-page') . '<iframe src="' . url('admin/appearance/styleguide/maintenance-page') . '" width="100%" height="400"></iframe>',

We should consider putting the width/height into CSS...

agentrickard’s picture

For a one-off iframe, is it really worth it? This code already runs through and alter hook if people need to change the behavior...

jessehs’s picture

Issue summary: View changes
Priority: Normal » Major
Status: Needs work » Needs review
FileSize
2.11 KB

Here's a new patch, rerolled, addressing some concerns. Working fieldgroups/javascript seems like a pretty major issue IMHO.

gmclelland’s picture

The patch in #9 applies and worked for me. Before the patch all javascript on the page was broken for me.

tanc’s picture

Status: Needs review » Reviewed & tested by the community

Confirmed patch in #9 fixed the broken javascript and allowed the vertical tabs to render correctly.

The last submitted patch, 1: 1004246-styleguide-vertical-tabs.patch, failed testing.

BWPanda’s picture

Status: Reviewed & tested by the community » Needs work

The patch (applied to the latest dev version) fixes the vertical tabs so that they display correctly, but doesn't display the maintenance page correctly - the page linked to from the Maintenance Page section, and the page displayed in the iframe, are copies of the current styleguide page, not the maintenance page...

loopduplicate’s picture

loopduplicate’s picture

loopduplicate’s picture

Hi All,

Regarding #13, I'm not able to reproduce "the Maintenance Page section, and the page displayed in the iframe, are copies of the current styleguide page". The Maintenance page section has admin/appearance/styleguide/maintenance-page for the link and the iframe. It is not the main styleguide page. If this is an issue, we need to file a separate report. The bug in this issue is the vertical tabs stopped working and there is a JS error on the page. That is fixed with #9.

I'm moving this back to RTBC. Please feel free to move it back if you think I'm wrong.

Regards,
Jeff

loopduplicate’s picture

Status: Needs work » Reviewed & tested by the community