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

New graph screen #169

Open
wants to merge 43 commits into
base: master
Choose a base branch
from
Open

New graph screen #169

wants to merge 43 commits into from

Conversation

jujinfu
Copy link
Collaborator

@jujinfu jujinfu commented Jan 28, 2015

No description provided.

panjiw and others added 30 commits February 23, 2014 03:52
Still very dirty but filtered data from selected view is now available (has been printed) not yet put into graphDisplay process though
…s there are no other functions other than graphing
@jujinfu jujinfu self-assigned this Jan 29, 2015
"usecurrentviewlist" => $screen->getVar('usecurrentviewlist')
);
include_once XOOPS_ROOT_PATH."/modules/formulize/include/graphdisplay.php";
displayGraph('Bar', $screen->getVar('fid'), $screen->getVar('frid'), $screen->getVar('labelelem'), $screen->getVar('dataelem'), $screen->getVar('ops'), $options);
Copy link
Collaborator

Choose a reason for hiding this comment

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

There's a problem here. These two variables are integers, which represent element IDs: $screen->getVar('labelelem'), $screen->getVar('dataelem'). Later in graphdislay.php, these are used like this: display($entry, $dataElement);. It works if the element has the same ID and handle, but if the element handle has been changed to text, such as, 'client_name' then it fails. The code in one of these places should be changed so that it's consistently using element IDs or handles, and it's probably better to use handles since that's what a lot of the other code does.

@jujinfu
Copy link
Collaborator Author

jujinfu commented Feb 5, 2015

at the beginning, all graphs are stored with label element id and data element id. And later it should be using element id any where else. I didn't notice this before. should I change all to handles? that means change from very beginning. or i just fix this by using id in other places?

I found the original display function there. It is using handles every where. how about i wrote a function that can convert ids and handles then call display only for graph screen.

Fixing this from beginning will take longer time. I would prefer fixing this from beginning, but is it worth to spend a lot of time to fix it rather than a small function doing the converting stuff?

I think at the beginning they were using element ids for graph screen. and when it comes to display function they didn't notice this difference too.

jujinfu added 2 commits February 5, 2015 23:08
In graphdisplay.php, using element handle instand of element id. in
graphScreen.php, convert element id to element handle before calling
dispalyGraph
@jujinfu
Copy link
Collaborator Author

jujinfu commented Feb 16, 2015

so far i have changed a lot in graphdisplay.php. now we will process all data then according to the graph type to generate the graph. still need some small improvements

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants