API page: http://api.drupal.org/api/drupal/includes--theme.inc/function/theme_prog...

It should be cleared that this theme function is only used for non-js batch pages.
Otherwise the theming of the progress bar will be handled by javascript (ajax) queries.

This information is not provided anywhere (?) but saves hours..

Comments

jhodgdon’s picture

Title: Documentation problem with theme_progress_bar » theme_progress_bar should have more information about when to use function
Version: 7.x-dev » 8.x-dev
Status: Active » Postponed (maintainer needs more info)
Issue tags: +Needs backport to D7

So are you saying that this is to be used for batch pages that are non-JS, or that it is used for non-batch pages?

patrickd’s picture

Title: theme_progress_bar should have more information about when to use function » Documentation problem with theme_progress_bar
Version: 8.x-dev » 7.x-dev
Status: Postponed (maintainer needs more info) » Needs review
Issue tags: -Needs backport to D7
StatusFileSize
new404 bytes
 /**
  * Returns HTML for an indentation div; used for drag and drop tables.
+ * Note that this is only used for non-javascript batch jobs.
  *
  * @param $variables
  *   An associative array containing:
patrickd’s picture

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

Sorry you posted, while I did^^

patrickd’s picture

Title: Documentation problem with theme_progress_bar » theme_progress_bar should have more information about when to use function
Issue tags: +Needs backport to D7

aww...

patrickd’s picture

OMG, commented the wrong function... -.- clearly too late!

This is the right one:

 /**
  * Returns HTML for a progress bar.
+ * Note that this is only used for non-javascript batch jobs.
  *
  * @param $variables
  *   An associative array containing:

Status: Needs review » Needs work

The last submitted patch, theme_progress_bar_documentation-1366740-5.patch, failed testing.

aspilicious’s picture

+++ b/includes/theme.incundefined
@@ -2104,6 +2104,7 @@ function theme_username($variables) {
  * Returns HTML for a progress bar.
+ * Note that this is only used for non-javascript batch jobs.

Needs a newline between those lines.

18 days to next Drupal core point release.

patrickd’s picture

you mean

 /**
  * Returns HTML for a progress bar.
  *
+ * Note that this is only used for non-javascript batch jobs.
  *
  * @param $variables
  *   An associative array containing:

?

aspilicious’s picture

patrickd’s picture

Status: Needs work » Needs review
StatusFileSize
new437 bytes

Sorry! This one hopefully..

jhodgdon’s picture

Status: Needs review » Postponed (maintainer needs more info)

I still have a question on this. Is there a reason why someone couldn't use this function to make a progress bar for other things besides batch jobs? And why can't this be used for batch jobs running under JS?

patrickd’s picture

Status: Postponed (maintainer needs more info) » Needs review

For shure, anyone can use this function to make his own progress bars.

Note that this is only used for non-javascript batch jobs.

batch.inc:
in function _batch_progress_page_js()
returned markup is hardcoded and simply <div id="progress"></div>
the progressbar markup will later by added by javascript without using theme_progress_bar

in function _batch_progress_page_nojs()
it uses return theme('progress_bar', array('percent' => $percentage, 'message' => $message));

Is there something you would change on the sentence?

jhodgdon’s picture

Status: Needs review » Needs work

OK. The wording that is in the patch now is:

Note that this is only used for non-javascript batch jobs.

That can be read as "You can only use this for batch jobs that are not JavaScript."

I think it needs to be reworded so that it means "You can use this for non-JavaScript batch jobs, or for things other than batch jobs", hopefully more succinctly.

Also, javascript should be capitalized properly as "JavaScript".

patrickd’s picture

Hmm lets say somone uses this theme function, I think he won't implement a new batch system..

The thing I really want to point out is that..

Drupal's core Batch API uses this only for non-JavaScript batch jobs.

That anyone can use this for his own stuff like a in-page progress bar (without using the batch API) should be clear anyway

you understand what I mean?^^

jhodgdon’s picture

That sounds like a good way to word the line you want to add. :)

patrickd’s picture

Status: Needs work » Needs review
StatusFileSize
new453 bytes

Okay, I hope this one is clear:

/**
 * Returns HTML for a progress bar.
 *
 * Note that the core Batch API uses this only for non-JavaScript batch jobs.
 *
 * @param $variables
 *   An associative array containing:
 *   - percent: The percentage of the progress.
 *   - message: A string containing information to be displayed.
 */
jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

That seems fine to me. I think having this in the docs is worthwhile, since themers will then be aware, if they override this function, what it will affect.

catch’s picture

Version: 8.x-dev » 7.x-dev
Status: Reviewed & tested by the community » Patch (to be ported)

This makes me wonder if we really need a dedicated hook_theme() for non-js progress bars at all, but good for the docs to be honest about how limited it is anyway.

Committed/pushed to 8.x, moving back to 7.x.

patrickd’s picture

Status: Patch (to be ported) » Needs review
StatusFileSize
new432 bytes

Yep, is there already a new implementation of the Batch API planned yet?

attached rerolled patch against d7

jhodgdon’s picture

Status: Needs review » Reviewed & tested by the community

Looks fine for d7, thanks!

webchick’s picture

Status: Reviewed & tested by the community » Fixed

Committed and pushed to 7.x. Thanks!

Status: Fixed » Closed (fixed)

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