-
Notifications
You must be signed in to change notification settings - Fork 29
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
code #418
base: main
Are you sure you want to change the base?
code #418
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,3 @@ | ||
Shoes.app do | ||
codes "Shoes.app do\n para 'Hello, world!'\nend" | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,14 @@ | ||
# frozen_string_literal: true | ||
|
||
module Shoes | ||
class Codes < Shoes::Drawable | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hm when i use code it giving me superclass mismatch idk if some other gem we using has it? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That would be weird, because it's under Shoes. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I don't get the superclass mismatch when I check out your branch. Maybe try a "git status" and see if you have any un-checked-in changes that might cause the problem? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is that why this PR isn't using There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It looks like we have a code TextDrawable already, called "code". So yeah, this is a bit odd - one called "codes", but also a regular Drawable not a TextDrawable. So if you declared it as Code, not Codes, you'd get the superclass error because Code is a child of Shoes::TextDrawable. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Should we remove the code text drawable, and make this PR There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @gintama91 btw this looks super cool :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It really does look awesome :-) It's true that we have no legacy examples using code(), only more recent ones. In Shoes3, looking at their source, it looks like code (grep for cCode) is a text type, so you can use it like em() inside a para(), and it's basically just monospaced like typewriter font in HTML. If we accept this, it should definitely be renamed to replace the other code element, not add ones named codes. |
||
shoes_styles :text | ||
|
||
def initialize(text) | ||
@text = text | ||
super | ||
|
||
create_display_drawable | ||
end | ||
end | ||
end |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
# frozen_string_literal: true | ||
|
||
module Scarpe::Webview | ||
class Codes < Drawable | ||
def initialize(properties) | ||
super | ||
end | ||
|
||
def element | ||
render("codes") | ||
end | ||
end | ||
end |
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.
@noahgibbs can you explain to me the differences?
Our examples don't have much use of this historically. And the manual says it is a text fragment
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.
In fact the only examples are ours. So, I'm possibly willing to go out on a limb for this feature? WDYT?
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.
The main difference is that one is a text fragment, like em() or strong() or sup(). It goes inside a para. So you'd do something like this:
Pavan's codes() here is a Drawable like para. It does formatting but doesn't really support other formatting inside it (unless we don't escape the HTML, but Shoes doesn't normally let you just put random HTML in places and have it format correctly.)
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.
Also, given that we now have the ability to add Scarpe-specific extensions, I think it should be one of those.