-
Notifications
You must be signed in to change notification settings - Fork 2
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
base: master
Are you sure you want to change the base?
Dev/gridstack #10
Conversation
Codecov Report
@@ 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.
|
There was a problem hiding this 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:
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:
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:
Also, in the settings Modals, there should be more padding. For reference, look at the picture below:
if(widgetX>29){ | ||
widgetY += 4; | ||
widgetX = 0; | ||
} |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What are these for?
There was a problem hiding this comment.
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}> |
There was a problem hiding this comment.
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
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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'> |
There was a problem hiding this comment.
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'>×</span> | |||
</button> | |||
</div> | |||
<div className='modal-body'> | |||
<div className='modal-b ody'> |
There was a problem hiding this comment.
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"> |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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, |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
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?
Implements moving widget on a grid