Add files via upload - Githubissues

vvidolov commented 7 years ago

Hi Amit,

We added the info for jqxGrid.

Let me know if you have any questions.

Best regards,

Victor Vidolov JQWidgets Sales sales@jqwidgets.com

AmitMY commented 7 years ago

Thank you for adding jqxGrid.

I would like some modifications if possible:

The demo link should be this one, as we are comparing the grid, and not the widgets package.
I am not sure, but I think a requirement for it is jQuery. If so, please replace Javascript with Jquery on frameworks. The Javascript tag means native JS, no additions required.
The idea behind adding links, and not just true/false is to also have some kind of centralised documentation. So please, if a feature is documented, link to that documentation page.
- for example, I saw you have pivoting from this file. I entered this link and did not find a pivoting documentation, even after a naive search. If it exists, but not documented, make it true. If it is documented, link to the correct documentation page. If it doesn't exist, it should be false
for keyboard navigation, I see you support "Arrows", "Enter", "Tab", "Page", "Home", "End" and not just "Arrows", "Enter", "Tab"

From a top view I could not find any "cells merging" or "pivoting" in the documentation. I did not look further

I will add those formats you mentioned to the tests, so it won't fail next time.

AmitMY commented 7 years ago

Another thing, is testing. You can locally run npm run test to see if the file is admissible. From this automated test, I can see that you have a rogue comma on line 34, breaking the JSON format.

vvidolov commented 7 years ago

Hi Amit,

I made the changes with your comments. I added some additional frameworks (you can change them if you like) and updated the pivot and the merge. We are bringing in a Pivot Grid with the next release and this is why I included the Pivot. As to the animations and the other features we have them as described by ag-grid, but not documented in this way.

Let me know if I need to fix anything else.

Thank you.

Victor

AmitMY commented 7 years ago

A step in the right direction indeed.

Same problem as before with the links. Every feature you mention must be true / false / Documentation link. All the links are the same, so it can be assumed that they do not describe the specific feature (look for example at ag-grid, handsontable, etc)

About the frameworks, it is indeed needed to remove some.

"ASP .NET MVC", // coding ideology "ASP .NET", // programming language "PHP", // programming language "JAVA", // programming language "TypeScript", // ~ programming language "RequireJS", // A modules loader "Bootstrap" // A design framework

The allowed frameworks are JS frameworks. Currently I know of: ["Javascript", "jQuery", "Angular1", "Angular2", "React", "Aurelia", "Web Components", "Vue"] And will add others if prompted.

vvidolov commented 7 years ago

Hi Amit,

I have done the features this way because they are best demonstrated in the demos not in the documentation. I have given the general link because our demos are called in such a way that if I give a link to a specific demo (like for example on grid filtering https://www.jqwidgets.com/jquery-widgets-demo/demos/jqxgrid/filtering.htm) the user experience is not so good as with the index.

Anyway, if you want it in this format I can rework it in the next days.

As to the frameworks, please remove what you don't like.

Thank you.

Victor Vidolov JQWidgets Sales

On Wed, Feb 15, 2017 at 2:37 PM, Amit Moryossef notifications@github.com wrote:

A step in the right direction indeed.

Same problem as before with the links. Every feature you mention must be true / false / Documentation link. All the links are the same, so it can be assumed that they do not describe the specific feature (look for example at ag-grid, handsontable, etc)

About the frameworks, it is indeed needed to remove some.

"ASP .NET MVC", // coding ideology "ASP .NET", // programming language "PHP", // programming language "JAVA", // programming language "TypeScript", // ~ programming language "RequireJS", // A modules loader "Bootstrap" // A design framework

The allowed frameworks are JS frameworks. Currently I know of: ["Javascript", "jQuery", "Angular1", "Angular2", "React", "Aurelia", "Web Components", "Vue"] And will add others if prompted.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AmitMY/grids/pull/2#issuecomment-279999973, or mute the thread https://github.com/notifications/unsubscribe-auth/ATldnvj8wCuKZphWwsC04czewiWlB8auks5rcvGKgaJpZM4MBm1- .

AmitMY commented 7 years ago

Yes. I think it would be better to have a direct link, or have no link at all, than having a link that does not fully help.

Also note, that line 41 has a rogue comma, breaking the JSON format. (so testing fails)

Thank you,

vvidolov commented 7 years ago

Thank you, I go that with the comma:)

I will rework the file in the next days and send it again.

I will not have enough time today.

Have a nice day.

Victor Vidolov JQWidgets Sales

On Wed, Feb 15, 2017 at 2:56 PM, Amit Moryossef notifications@github.com wrote:

Yes. I think it would be better to have a direct link, or have no link at all, than having a link that does not fully help.

Also note, that line 41 has a rogue comma, breaking the JSON format. (so testing fails)

Thank you,

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AmitMY/grids/pull/2#issuecomment-280003704, or mute the thread https://github.com/notifications/unsubscribe-auth/ATldnvNDiBkxGr9A-EfPly0DdqnnqkYqks5rcvYTgaJpZM4MBm1- .

AmitMY commented 7 years ago

Thank you for the update, this is about done.

You can see from the tests

not ok 576 Grid's features visualisation must be an array or null

Visualisation means graphs, and charts. If you have graphs/charts, the array must be a subset of: ["Line", "Bar", "H-Bar", "Map", "Progress", "Pie", "Radar", "Custom"]

vvidolov commented 7 years ago

Thanks Amit,

We have a chart component with all those charts (and more) and the grid can show a chart in the cells.

Should I just put all the chart types we have?

Best regards,

Victor Vidolov JQWidgets Salas www.jqwidgets.com

On Mon, Feb 20, 2017 at 3:52 PM, Amit Moryossef notifications@github.com wrote:

Thank you for the update, this is about done.

You can see from the tests https://travis-ci.org/AmitMY/grids/builds/203444189

not ok 576 Grid's features visualisation must be an array or null

Visualisation means graphs, and charts. If you have graphs/charts, the array must be a subset of: ["Line", "Bar", "H-Bar", "Map", "Progress", "Pie", "Radar", "Custom"]

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub https://github.com/AmitMY/grids/pull/2#issuecomment-281084432, or mute the thread https://github.com/notifications/unsubscribe-auth/ATldnvQI5yYhcPW3R4xUf-RMmvCinwcAks5reZqIgaJpZM4MBm1- .

AmitMY commented 7 years ago

Yeah, put all the charts you have, and I will add those to valid types

AmitMY / grids

Add files via upload #2