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

First commit. Module is in D8 structure #14

Draft
wants to merge 8 commits into
base: 7.x-1.x
Choose a base branch
from
Draft

Conversation

reynoldtan
Copy link
Contributor

@reynoldtan reynoldtan commented Mar 16, 2021

DO NOT MERGE

This PR has been created to show the status of the Tripal 4 upgrade of this module. It should be used for testing purposes!

The 9.x-2.x branch should NEVER be merged into the current master branch

Description

This PR will make Tripal D3 module compatible with Drupal 8.

Testing?

  • I tested on a generic Tripal Site and Drupal 8
  • I tested on a KnowPulse Clone
  • This PR includes automated testing
  1. Before installing this module, it is important that the main library required is present in your libraries for external libraries.
    https://github.com/d3/d3/releases/download/v3.5.14/d3.zip
    Download D3, unpack to sites/all/assets/vendor/d3 directory.

  2. Install and enable module.

  3. At the moment, Tripal does not consolidate all Tripal extension modules under yoursite/admin/tripal/extension. To view configuration and demo page of this module, use the link below
    yoursite/admin/tripal/extension/tripald3

Documentation has been updated.

Documentation on how to create simple pie, simple donut and multi-series donut has been revised to comply with Drupal 8 requirements. A completed module that show the final result of following the step by step guide can be enabled to view the outcome.

yoursite/mychart/simplepie,
yoursite/mychart/simpledonut,
yoursite/mychart/multiseriesdonut,
and
yoursite/admin/tripal/extension/tripald3 - This module implements a hook that will extend colour scheme (titled monochromatic).

See my_chart

Unit test.

A unit test (Browser Test) in Tests/src/Functional/ is available and can be executed using Drupal 8 in yoursite/admin/config/development/testing search for tripalD3 module.

Copy link
Member

@laceysanderson laceysanderson left a comment

Choose a reason for hiding this comment

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

  • Do not delete the License and the Readme. Furthermore, the Readme should be updated to reflect the Tripal Version and should follow the ND Genotypes module template.
  • API's should be converted to Drupal services where it makes sense to do so. At a minimum, they should become class-based.
  • Remove all .DS_Store files you've committed.
  • RST files should use .. code-block:: langcode not .. code:: for the reliable rendering of syntax colouring.
  • ensure you always have newlines at the bottom of a file.
  • Setup automated testing on this repository as done in Tripal4
  • Upgrade-info.html should not be committed.

@@ -110,21 +109,19 @@ function tripald3_get_scheme_colors($scheme_id, $type, $scheme = array()) {
/**
* Register Color Schemes with Javascript
*/
function tripald3_register_colorschemes() {
function tripald3_register_colorschemes($default) {
Copy link
Member

Choose a reason for hiding this comment

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

This is a non-backwards compatible change to the API. You need to provide a default for $default to minimize changes needed for others to upgrade their modules.

$jsSettings['tripalD3']['colorSchemes'][$id]['name'] = $scheme['name'];
$jsSettings['tripalD3']['colorSchemes'][$id]['quantitative'] = tripald3_get_scheme_colors($id, 'quantitative', $scheme);
$jsSettings['tripalD3']['colorSchemes'][$id]['categorical'] = tripald3_get_scheme_colors($id, 'categorical', $scheme);
$jsSettings['colorSchemes'][$id]['name'] = $scheme['name'];
Copy link
Member

Choose a reason for hiding this comment

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

Why did you drop the tripalD3 key here? This was a Drupal standard to reduce the chance of collisions in js settings between modules.

api/pedigree.inc Outdated
}

return $rels_to_restrict;
}
Copy link
Member

Choose a reason for hiding this comment

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

Should have a newline at the end of files.

Suggested change
}
}

@laceysanderson laceysanderson marked this pull request as draft August 6, 2021 16:26
@laceysanderson laceysanderson linked an issue Aug 6, 2021 that may be closed by this pull request
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.

Upgrade to Tripal 4 + Drupal 9
2 participants