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

Handling forms within live. #6

Open
wants to merge 11 commits into
base: main
Choose a base branch
from
Open

Conversation

pharmac1st
Copy link

Description

As required by the issue, a new class Live::Form has been created for the server side handling of forms, it is a child function of Live::View. Live::Form provides a new class variable @recieved_form which is a serialized hash version of the form data. This PR also comes with a fix for the details argument not being passed properly. Two new javascript Live class functions are added for clientside handling of forms, see the live-js PR.

Types of Changes

  • New feature.

Testing

I tested my changes locally.

@pharmac1st pharmac1st mentioned this pull request Jul 5, 2021
lib/live/form.rb Outdated
class Form < Live::View

#returns [String] javascript code to execute when the form is submitted.
def handleForm(details=false)
Copy link
Member

@ioquatix ioquatix Jul 5, 2021

Choose a reason for hiding this comment

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

We should probably call this forward_submit.

Copy link
Author

Choose a reason for hiding this comment

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

lib/live/form.rb Outdated Show resolved Hide resolved
lib/live/form.rb Outdated
end
end

#Render the element
Copy link
Member

Choose a reason for hiding this comment

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

Please include a space between # and the comment, e.g.

# Render the form element.

The returns line is probably not needed.

Copy link
Author

Choose a reason for hiding this comment

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

lib/live/form.rb Outdated
#Handles an incoming event.
# @parameter load [String] the parsed message from the view layer,
# which includes the details and the serialized form data.
def handle(event, message)
Copy link
Member

Choose a reason for hiding this comment

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

I believe you should introduce a new method above called def submit(form_data)

This method should then invoke that with the argument rather than setting an instance variable.

The sub-class can be responsible for updating data and/or form data. Maybe we should repurpose @data - i.e. fields that are stored in the form are serialized as part of the form itself anyway, so it doesn't need to be part of the data- attributes. Not sure best way.

lib/live/page.rb Outdated
@@ -99,7 +99,7 @@ def run(connection)
Console.logger.warn(self, "Could not resolve element:", message)
end
elsif id = message[:id]
self.handle(id, message[:event])
self.handle(id, message[:event], message[:event][:details])
Copy link
Member

Choose a reason for hiding this comment

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

Is this necessary? I believe the message[:event] can be decoded by the handle method.

Copy link
Author

Choose a reason for hiding this comment

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

@ioquatix
Copy link
Member

ioquatix commented Jul 5, 2021

It looks good to me so far, I've given you some feedback.

@ioquatix
Copy link
Member

ioquatix commented May 4, 2024

@pharmac1st do you want to try reworking this PR on the latest version of Live.js? I've merged form data handling.

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