Skip to content

Add support for classes to isometric objects.#217

Merged
elchininet merged 8 commits intoelchininet:masterfrom
joelburget:classes
Dec 22, 2024
Merged

Add support for classes to isometric objects.#217
elchininet merged 8 commits intoelchininet:masterfrom
joelburget:classes

Conversation

@joelburget
Copy link
Copy Markdown
Contributor

Fixes #216

@elchininet
Copy link
Copy Markdown
Owner

Hi @joelburget,
Thanks for contributing 🙂
Some things that I have observed in a quick look, please, fix them and I'll review the code with more attention:

  1. The className should not have an empty string as a default value, this parameter should be added only if it has a value. The snapshots that are not using that option should not have an empty class parameter.
  2. There should be tests cases for this new option
  3. This new option should be added to the documentation to each class

Regards and thanks again for contributing to the project.

@elchininet elchininet added the enhancement New feature or request label Dec 20, 2024
…s parameter should be added only if it has a value. The snapshots that are not using that option should not have an empty class parameter.
@joelburget
Copy link
Copy Markdown
Contributor Author

I think I fulfilled all of your requests, please let me know how it looks.

@elchininet
Copy link
Copy Markdown
Owner

I think I fulfilled all of your requests, please let me know how it looks.

@joelburget, looks good at first sight. I'll take a deeper look when I have some time. 👍

Copy link
Copy Markdown
Owner

@elchininet elchininet left a comment

Choose a reason for hiding this comment

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

LGTM, just some removals are required.

@elchininet
Copy link
Copy Markdown
Owner

@joelburget,
It is ready to merge, rebase with master and solve the conflicts with dist/web/isometric.js.

@joelburget
Copy link
Copy Markdown
Contributor Author

I just merged and rebuilt dist/web/isometric.js (git p --no-rebase upstream master; npm run build; git add dist/web/isometric.js)

@coveralls
Copy link
Copy Markdown

Coverage Status

coverage: 100.0%. remained the same
when pulling 0b7554c on joelburget:classes
into defa492 on elchininet:master.

@elchininet elchininet merged commit 43c416d into elchininet:master Dec 22, 2024
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.

Support CSS Classes?

3 participants