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

Dev/gridstack #10

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

Dev/gridstack #10

wants to merge 29 commits into from

Conversation

nicholasunderwood
Copy link

Implements moving widget on a grid

@codecov
Copy link

codecov bot commented Jan 19, 2019

Codecov Report

Merging #10 into master will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master      #10   +/-   ##
=========================================
  Coverage     33.21%   33.21%           
  Complexity       28       28           
=========================================
  Files             8        8           
  Lines           277      277           
  Branches         41       41           
=========================================
  Hits             92       92           
  Misses          173      173           
  Partials         12       12

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update b5d4057...9d9f854. Read the comment docs.

@nicholasunderwood nicholasunderwood removed the request for review from BBScholar January 19, 2019 01:52
Copy link
Member

@andycate andycate left a comment

Choose a reason for hiding this comment

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

This looks really good so far. There are definitely things that need to be added though. First off, when widgets are moved around, the final arrangement and size of widgets should be saved in the config settings. Second, when the window is resized, the widgets should stay the same size, but wrap accordingly. Right now, I see this when I resize the page:
screen shot 2019-01-28 at 6 30 53 pm

Also, I feel like there should be a button in the title bar of each widget that is explicitly for clicking and dragging. For example here:
screen shot 2019-01-28 at 6 33 17 pm

Also, I feel like the padding around the widgets should be smaller. And it should be constant when the page is resized. The part in red should be smaller:
screen shot 2019-01-28 at 6 35 40 pm

Also, in the settings Modals, there should be more padding. For reference, look at the picture below:
screen shot 2019-01-28 at 6 43 40 pm

src/main/resources/page.html Outdated Show resolved Hide resolved
if(widgetX>29){
widgetY += 4;
widgetX = 0;
}
Copy link
Member

Choose a reason for hiding this comment

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

What are these arbitrary numbers doing here?

Copy link
Author

Choose a reason for hiding this comment

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

If their are more elements than space on the page, they will warp to the next row

@@ -1,12 +1,21 @@
var widgetX = -7;
var widgetY = -4;
Copy link
Member

Choose a reason for hiding this comment

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

What are these for?

Copy link
Author

Choose a reason for hiding this comment

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

It's used to arrange the widgets in a line

<div className='card-header p-1'>
<h4 className='m-0 d-inline'>{this.props.title}</h4>
<button className='btn btn-light float-right d-inline p-0 m-1' type='button' data-toggle='modal' data-target={'#' + this.props.id + '_modal'}><h5 className='fas fa-cog m-0'></h5></button>
<div className='card m-1 grid-stack-item' style={{display:'inline-block'}} id={this.props.id + '_card'} data-gs-no-resize={this.props.noResize} data-gs-width={this.props.width} data-gs-height={this.props.height} data-gs-y={widgetY} data-gs-x={widgetX}>
Copy link
Member

Choose a reason for hiding this comment

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

widgetY and widgetX should be properties that are stored in the config JSON

Copy link
Member

Choose a reason for hiding this comment

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

Also, it looks like the noresize thing isn't working. I commented on it further down too.

Copy link
Author

Choose a reason for hiding this comment

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

This is done automatically to lay them out in a line. I feel like user input could create user error.

<button className='btn btn-light float-right d-inline p-0 m-1' type='button' data-toggle='modal' data-target={'#' + this.props.id + '_modal'}><h5 className='fas fa-cog m-0'></h5></button>
<div className='card m-1 grid-stack-item' style={{display:'inline-block'}} id={this.props.id + '_card'} data-gs-no-resize={this.props.noResize} data-gs-width={this.props.width} data-gs-height={this.props.height} data-gs-y={widgetY} data-gs-x={widgetX}>
<div className='grid-stack-item-content'>
<div className='card-header p-1 grid-stack-item-content ui-draggable-handle'>
Copy link
Member

Choose a reason for hiding this comment

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

Make a button inside the header that allows for clicking and dragging

@@ -34,7 +43,7 @@ class WidgetSettings extends React.Component {
<span aria-hidden='true'>&times;</span>
</button>
</div>
<div className='modal-body'>
<div className='modal-b ody'>
Copy link
Member

Choose a reason for hiding this comment

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

fix this. This might also fix the padding issue.

@@ -52,18 +61,26 @@ $(function() { // runs when document finishes loading
if(PageUtils.loadPageConfig()) {
SocketHandler.connect(PageUtils.getWebSocketPageAddress());
ReactDOM.render(
<div>
<div className="grid-stack" data-gs-width="35">
Copy link
Member

Choose a reason for hiding this comment

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

I think that the width should be varied based on the browser window size. Not sure though

{PageUtils.renderWidgets()}
</div>,
$('#reactapp')[ 0 ]
);
$(".grid-stack").gridstack({
width: 35,
Copy link
Member

Choose a reason for hiding this comment

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

Should this also be varied?

"id": "testVarEditor",
"kwargs": {},
"resize": false,
Copy link
Member

Choose a reason for hiding this comment

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

This functionality seems to be broken because I can resize this widget.

Copy link
Author

Choose a reason for hiding this comment

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

This works on my machine. If you get a chance can you check dev tools and make sure there is a data-gs-no-resize property?

@andycate andycate added the enhancement New feature or request label Jan 29, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants