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.

Sign up for GitHub

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

Make parent formid a part of autogenerated oid #120

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

Make parent formid a part of autogenerated oid #120

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
5100c02
A counter is no longer required for multiple forms
ezag Oct 22, 2021
dcd2668
Improve the comment wording
ezag Oct 23, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 7 additions & 15 deletions deformdemo/__init__.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2550,35 +2550,27 @@ class Schema(colander.Schema):
@view_config(renderer="templates/form.pt", name="multiple_forms")
@demonstrate("Multiple Forms on the Same Page")
def multiple_forms(self):
import itertools

# We need to make sure the form field identifiers for the two
# forms do not overlap so accessibility features continue to work,
# such as focusing the field related to a legend when the
# legend is clicked on.
# We do so by creating an ``itertools.count`` object and
# passing that object as the ``counter`` keyword argument to
# the constructor of both forms. As a result, the second
# form's element identifiers will not overlap the first
# form's.

counter = itertools.count()
# We do so by passing distinct ``formid`` keyword argument to the
# constructor of both forms. As a result, form's element identifiers
# will inherit parent's ``formid`` and will not overlap.
# (Note: alternatively, we could create an ``itertools.count`` object
# and pass that object as the ``counter`` keyword argument.)
Copy link
Member

Choose a reason for hiding this comment

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

Your English is better than my Russian. 😉 I have only a couple of corrections.

        # We do so by passing distinct ``formid`` keyword arguments to the
        # constructor of both forms.  As a result, the form's element identifiers
        # will inherit the parent's ``formid`` and will not conflict.
        # (Note: alternatively, we could create an ``itertools.count`` object
        # and pass that object as the ``counter`` keyword argument.)

Also after thinking about this, it would be good to demonstrate both examples of having two forms in a page. In other words, (1) let's keep the new method of explicitly providing the formid, and (2) would you please add the alternative example as you describe in your note?

That will require another test for the new widget, but I think it's worth doing.

Copy link
Author

Choose a reason for hiding this comment

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

I think that for an unfamiliar person it would be a bit misleading to have an example with the extra counter related code that could be removed with no apparent effect: setting distinct formids is a must anyway, and altered oids don't seem to matter from the example's standpoint.

That said, it is definitely worth having an example that demonstrates the counter feature; can you think of an appropriate one? And if not, do you think we can consider deprecating and/or removing the counter feature at all, in order to spare further code maintenance efforts?


class Schema1(colander.Schema):
name1 = colander.SchemaNode(colander.String())

schema1 = Schema1()
form1 = deform.Form(
schema1, buttons=("submit",), formid="form1", counter=counter
)
form1 = deform.Form(schema1, buttons=("submit",), formid="form1")

class Schema2(colander.Schema):
name2 = colander.SchemaNode(colander.String())

schema2 = Schema2()
form2 = deform.Form(
schema2, buttons=("submit",), formid="form2", counter=counter
)
form2 = deform.Form(schema2, buttons=("submit",), formid="form2")

html = []
captured = None
Expand Down
12 changes: 6 additions & 6 deletions deformdemo/test.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3498,22 +3498,22 @@ class MultipleFormsTests(Base, unittest.TestCase):

def test_render_default(self):
self.assertEqual(
findid_view("deformField1").get_attribute("name"), "name1"
findid_view("form1Field1").get_attribute("name"), "name1"
)
self.assertEqual(
findid_view("deformField1").get_attribute("value"), ""
findid_view("form1Field1").get_attribute("value"), ""
)
self.assertEqual(findid("deformField3").get_attribute("name"), "name2")
self.assertEqual(findid("deformField3").get_attribute("value"), "")
self.assertEqual(findid("form2Field1").get_attribute("name"), "name2")
self.assertEqual(findid("form2Field1").get_attribute("value"), "")
self.assertRaises(NoSuchElementException, findcss, ".is-invalid")

def test_submit_first(self):
findid("deformField1").send_keys("hey")
findid("form1Field1").send_keys("hey")
findid("form1submit").click()
self.assertEqual(eval(findid("captured").text), {"name1": "hey"})

def test_submit_second(self):
findid("deformField3").send_keys("hey")
findid("form2Field2").send_keys("hey")
findid("form2submit").click()
self.assertEqual(eval(findid("captured").text), {"name2": "hey"})

Expand Down