-
Notifications
You must be signed in to change notification settings - Fork 120
Implement converting PageContent's into HTML #231
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
base: 1.20
Are you sure you want to change the base?
Conversation
Implemented IHTML for everything in Materials and You Took 1 minute
Took 15 minutes
Took 1 hour 54 minutes
…t formatting system Took 1 hour 57 minutes
Took 2 hours 12 minutes
Took 14 minutes
Took 9 minutes
Took 38 seconds
|
Its also worth asking, do you plan to implement tooltips or not? Its fine without tooltip; text + links is better than nothing. But want to know so I know how to review this. |
src/main/java/slimeknights/mantle/client/screen/book/BookScreen.java
Outdated
Show resolved
Hide resolved
src/main/java/slimeknights/mantle/client/screen/book/BookScreen.java
Outdated
Show resolved
Hide resolved
src/main/java/slimeknights/mantle/client/screen/book/TextDataRenderer.java
Outdated
Show resolved
Hide resolved
|
|
Removed unnecessary spans and colors Took 3 hours 23 minutes
Took 1 hour 45 minutes
src/main/java/slimeknights/mantle/client/book/data/content/ContentTextImage.java
Outdated
Show resolved
Hide resolved
src/main/java/slimeknights/mantle/command/client/BookCommand.java
Outdated
Show resolved
Hide resolved
src/main/java/slimeknights/mantle/client/book/data/element/TextData.java
Outdated
Show resolved
Hide resolved
| Mantle.logger.warn("{} does not implement IHTML. If it does not contain text, ignore this warning.", this.getClass()); | ||
| return "<p>" + this.getClass() + "</p>"; |
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 believe this default implementation is redundant and is probably better left out, since it is implemented on the base PageContent, and any subsequent child content inherits that.
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 do want to keep this though, it makes figuring out which class to work on very easy
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 might be worth making it abstract in the interface, but then put this default implementation in PageContent. Are there any other classes that implement the interface without providing an implementation?
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 removed the implementation in PageContent, so everything else I haven't changed
src/main/java/slimeknights/mantle/client/book/data/content/ContentBlockInteraction.java
Outdated
Show resolved
Hide resolved
Took 12 minutes
Took 9 minutes
Took 15 minutes
Took 11 minutes
Added another title helper Took 30 minutes
Took 39 minutes
Took 23 minutes
Took 1 hour 55 minutes
# Conflicts: # src/main/java/slimeknights/mantle/client/screen/book/BookScreen.java
Took 2 hours 8 minutes
Took 1 hour 1 minute
Took 2 hours 37 minutes
Took 42 minutes
Took 10 minutes
Took 35 minutes
Took 44 minutes Took 5 minutes
Took 31 minutes
Took 1 hour 57 minutes
# Conflicts: # src/main/java/slimeknights/mantle/client/screen/book/BookScreen.java # src/main/java/slimeknights/mantle/command/client/BookCommand.java Took 15 seconds
Took 44 minutes
Took 29 minutes Took 24 seconds
SlimeKnights/slimeknights.github.io#16