Text input for the type of chart to render can lead to errors in selection. This patch does the following:

  • Defines a run time defined value for the current default value
  • Points examples to use the default method (if they were previously hard coded)
  • Creates a function that returns an array of potential chart rendering methods
  • Creates a hook/alter for modifying the supplied list of chart rendering methods

Using the hook/alter method the Orgchart integration could be split out into it's own module for example since it's a very different kind of data set to render then the other types of displays.

CommentFileSizeAuthor
#5 betterrenderdefinition-1803618-5.patch5.79 KBbtopro
moreflexiblerenderingdefinition.patch5.72 KBbtopro

Comments

konforti’s picture

konforti commented

Yes, this is looks like a good idea.
About the hook_define_gct_type() and hook_define_gct_type_alter(),
Are they both needed?

As I see it, the basic chart types can be hard code in the module with an hook_define_gct_type_alter() to alter it by other modules.
What is the case of using the hook_define_gct_type() out of the implementation you made in the module?

About the Orgchart integration, you right it's very different. most of differences are in the JS file.
Its can be a good idea to split it out to a sub-module.

konforti’s picture

konforti commented
Status: Needs review » Needs work

The comments on the hook_define_gct_type_alter() wrong and says:
"Implements hook_define_gct_type()."
And one more thing, the hooks definitions should added in the google_chart_tools.api.php file.

btopro’s picture

btopro commented

Not sure that there's a real reason to have both hook and alter, I think it's more so the convention of true definition / concatenation vs modification of previous results. That way the sub-module (theoretically) is providing it's definition to the lot as opposed to altering it's definition in (minor difference). Both of those sound right and very minor, do you want me to reroll a patch with those included?

konforti’s picture

konforti commented

Yes sure, this will be great.

btopro’s picture

btopro commented
Version: 7.x-1.3 » 7.x-1.x-dev
Status: Needs work » Needs review
StatusFileSize
new betterrenderdefinition-1803618-5.patch5.79 KB

Reroll of patch, changes made from original:

  • without the originating hook, now only the alter hook exists to modify
  • Changed the name slightly to reflect the lack of an originating hook
  • Added example usage to the api file instead of in the module file
  • fixed comment typo
konforti’s picture

konforti commented
Status: Needs review » Fixed

Committed: http://drupalcode.org/project/google_chart_tools.git/commitdiff/6aa63954...

Thanks.

Status: Fixed » Closed (fixed)

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