Skip to content

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

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

</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.

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

$('#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?

"height": "3",
"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.

3 participants