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

Finally implementing Grid #93

Merged
merged 24 commits into from
Mar 31, 2017
Merged

Finally implementing Grid #93

merged 24 commits into from
Mar 31, 2017

Conversation

romaninsh
Copy link
Member

@romaninsh romaninsh commented Mar 30, 2017

Merge #74 first.

This PR implements a new Grid class with a bunch of features:

  • menu / button support on top of the grid
  • quick-search input field
  • support for checkboxes
  • support for actions (buttons)

screen shot 2017-03-30 at 11 05 35

@romaninsh
Copy link
Member Author

I must also add that this PR fixes HTML-escaping in Table.

@romaninsh romaninsh mentioned this pull request Mar 30, 2017
$this->setAttr('href', $this->app->url($url));

return $this;
}
}
Copy link
Member Author

Choose a reason for hiding this comment

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

moved to View

@romaninsh
Copy link
Member Author

Actually - docs will come in the final implementation of Grid, which I'm yet to PR.

@DarkSide666 DarkSide666 self-requested a review March 31, 2017 09:44
fixed this once few days ago, but it appeared again :)
Copy link
Member

@DarkSide666 DarkSide666 left a comment

Choose a reason for hiding this comment

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

Something is wrong with multiple formatters in columns.
Besides that all looks great!


$output[] = $column->getCellTemplate($field);
// If multiple formatters are defined, use the first for the header cell
Copy link
Member

Choose a reason for hiding this comment

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

If data cell has multiple formatters, then all of them should be used here. This is not header cell here !

Copy link
Member Author

Choose a reason for hiding this comment

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

I'm unclear how to implement that. I need more info in #92 and #67. The mechanism is there, so I propose we just look for bugs and various combinations then address them.

*/
public function jsChecked()
{
return new \atk4\ui\jsExpression(' $('.$this->table->jsRender().").find('.checked.".$this->class."').closest('tr').map(function(){ ".
Copy link
Member

Choose a reason for hiding this comment

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

can't we use simpler JS syntax here? something like:

$(...).find('.checked.xxx').closest('tr').get(0).data('id');

Copy link
Member Author

Choose a reason for hiding this comment

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

I'll refactor those later with JSLib. Will create a separate issue.

@DarkSide666 DarkSide666 merged commit 36eeae7 into develop Mar 31, 2017
@DarkSide666 DarkSide666 deleted the feature/grid-part3 branch March 31, 2017 10:53
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.

2 participants