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

Mark Summary and Details as block items #1276

Merged
merged 3 commits into from
May 26, 2023

Conversation

arjanmels
Copy link
Contributor

This fixes issue #1275 and also adds a test case for it.

Copy link
Owner

@Sub6Resources Sub6Resources left a comment

Choose a reason for hiding this comment

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

Looks good, just one thing that needs to get changed to make it all work

styledElement.style = Style(
display: Display.block,
);
break;
Copy link
Owner

Choose a reason for hiding this comment

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

This style needs to be added to DetailsElementBuiltIn in the prepare method, rather than this class.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're sort of right in the StyledElementBuiltin it indeed doesn't do anything. However if I add it into the DetailsElementBuiltin it causes different rendering issues. I removed it for Summary and left it in just for Details; this seems to work properly.

@Sub6Resources Sub6Resources added the bugfix Fixes a bug in the package label May 23, 2023
Copy link
Owner

@Sub6Resources Sub6Resources left a comment

Choose a reason for hiding this comment

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

Sounds good and looks good. Thanks!

@Sub6Resources Sub6Resources merged commit a380ebc into Sub6Resources:master May 26, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bugfix Fixes a bug in the package
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants