Skip to content

Conversation

@lacroixdavid1
Copy link
Contributor

Description

Please explain the changes you made here. Include an example of what your changes fix or how to use the changes.

Checklist

  • Created tests which fail without the change (if possible) (N/A)
  • All tests passing (yarn test) (N/A)
  • Extended the README / documentation, if necessary

#1823

### Usage

```typescript
gridstackConfig: GridStackOptions = {
Copy link
Member

Choose a reason for hiding this comment

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

this is very specific to your needs... might not be best generic example, but easy enough for me to cleanup.

Copy link
Contributor Author

@lacroixdavid1 lacroixdavid1 Aug 8, 2021

Choose a reason for hiding this comment

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

this is very specific to your needs... might not be best generic example, but easy enough for me to clean up.

Like it's mentionned in the README.md caveat section


@Input() public x: number;
@Input() public y: number;
@Input() public width: number;
Copy link
Member

Choose a reason for hiding this comment

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

I intentionally did not use width/height to conflict with DOM existing attributes...

Copy link
Contributor Author

@lacroixdavid1 lacroixdavid1 Aug 8, 2021

Choose a reason for hiding this comment

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

It's not an issue. Using [height]="42" won't collide with the native attributes. If the user wants to set native height attribute, he would do [attr.height.px]="42"


@Input() public options: GridStackOptions;

@Output() public gridstackAdded = new EventEmitter<{
Copy link
Member

@adumesny adumesny Aug 7, 2021

Choose a reason for hiding this comment

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

those should all be typed since they are the same - not clear why you need to pass Gridstack again (caller would have it).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The caller might need the grid reference to get all the current elements and their position in it to save the user's customization. That's why I'm doing in my app.

@lacroixdavid1
Copy link
Contributor Author

Closing. A bit like you on #1820, I don't have time for this. It's up to you if you want to reopen, adjusts, and merge.

@adumesny adumesny reopened this Aug 8, 2021
@thoussem
Copy link

Hello @lacroixdavid1,
Do you face an issue with nested grid with your angular wrapper ?

@adumesny adumesny merged commit 21167d3 into gridstack:master Dec 28, 2022
adumesny added a commit to adumesny/gridstack.js that referenced this pull request Dec 28, 2022
* better version of gridstack#1824
* moved classes to correct place, simplified htm + css to be in .ts template (only 2 files to copy now)
* fixed the incorrect event handlers to pass the right set of params for each, cleanup
* grid-item simpler elements (matches Nodes fields)

MORE TODO....
@adumesny adumesny mentioned this pull request Dec 28, 2022
3 tasks
adumesny added a commit to adumesny/gridstack.js that referenced this pull request Dec 29, 2022
better gridstack#1824
* we now have a much cleaner and optimized Ng wrapper, as well as documented README file
* this needs fixes in the lib (upcoming release) to work correctly
@adumesny adumesny mentioned this pull request Dec 29, 2022
3 tasks
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.

3 participants