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

para text #395

Merged
merged 1 commit into from
Oct 5, 2023
Merged

para text #395

merged 1 commit into from
Oct 5, 2023

Conversation

gintama91
Copy link
Collaborator

Description

para text, as it's functionality seems same as replace i just updated code for that

Image(if needed, helps for a faster review)

image

Checklist

  • Run tests locally
  • Run linter(check for linter errors)

@@ -0,0 +1,14 @@
Shoes.app do
stack do
@note = para " your note will appear here"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Would be good to fix the indentation here.

@@ -27,10 +27,18 @@ def initialize(*args, stroke: nil, size: :para, font: nil, hidden: false, **html
def text_children_to_items(text_children)
text_children.map { |arg| arg.is_a?(String) ? arg : arg.linkable_id }
end
def replace(*children)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Also funny indentation here.


def replace(*children)
@text_children = children
def text=(*children)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Will this ever be multiple items? I think an assignment will always just get one item. Not sure how this would work if you assigned an array.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

hm , ok n also in replace does it make diff if we use splat operator or no?

Copy link
Collaborator

Choose a reason for hiding this comment

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

Looks like sometimes it does?

noah@Noahs-MBP-3 scarpe % irb
irb(main):001> class Bobo; def text=(val); @text=val; end; def tsplat=(*args); @tsplat = args; end; end
=> :tsplat=
irb(main):002> b = Bobo.new
=> #<Bobo:0x00000001045f0c30>
irb(main):003> class Bobo; attr_reader :text; attr_reader :tsplat; end
=> [:tsplat]
irb(main):004> b.text
=> nil
irb(main):006> b.text = *[1, 2, 3]
=> [1, 2, 3]
irb(main):007> b.text
=> [1, 2, 3]
irb(main):008> b.tsplat = *[1, 2, 3]
=> [1, 2, 3]
irb(main):009> b.tsplat
=> [[1, 2, 3]]

Copy link
Collaborator Author

@gintama91 gintama91 Sep 29, 2023

Choose a reason for hiding this comment

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

hm okay, but why do we/user pass array in either replace or in text...,(yeah there might be ppl who do but in that case should we try to convert array into string here or something?)
as of now even if we use splat operator if user passes anything other than string it just raises error if it dont have linkable id...

something like ?

 if arg.is_a?(String)
   arg
 elsif arg.is_a?(Array)
   arg
 else
   arg.linkable_id
 end

Copy link
Collaborator

Choose a reason for hiding this comment

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

Normally you wouldn't use a splat operator in an assignment. If the user wants to assign an array, they can just assign the array value. Something like:

def text=(val)
  @text=val
end

# or more succinctly:

attr_writer(:text)

Copy link
Collaborator

@noahgibbs noahgibbs Sep 29, 2023

Choose a reason for hiding this comment

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

Though never mind, attr_writer is no good because we need to make sure text gets sent via the text_childrentext_items display property.

@gintama91
Copy link
Collaborator Author

sorry about indentation,i forgot to install vs code extension for it after reinstalling kubuntu

@gintama91
Copy link
Collaborator Author

changed this

@noahgibbs noahgibbs merged commit b97bf01 into scarpe-team:main Oct 5, 2023
1 check passed
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.

2 participants