Skip to content

[change] Allow multiple jsonschema widgets in one page#333

Closed
nemesifier wants to merge 1 commit into
masterfrom
improve-jsonwidget
Closed

[change] Allow multiple jsonschema widgets in one page#333
nemesifier wants to merge 1 commit into
masterfrom
improve-jsonwidget

Conversation

@nemesifier

Copy link
Copy Markdown
Member

Earlier the schema.json was bound to each view. No matter how many
different switcher were present, it was only loading one of them.

With this patch, fetching schema.json is bound to each switcher element.
This allows fetching different JSONSchemas on the same page.

Earlier the schema.json was bound to each view. No matter how many
different switcher were present, it was only loading one of them.

With this patch, fetching schema.json is bound to each switcher element.
This allows fetching different JSONSchemas on the same page.

@nemesifier nemesifier left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@pandafy I found a minor issue while testing the widget in the credentials edit page, can you replicate it?

Screenshot from 2020-12-04 10-42-35

@coveralls

Copy link
Copy Markdown

Coverage Status

Coverage increased (+0.0008%) to 99.174% when pulling a53c16c on improve-jsonwidget into fcfa337 on master.

@pandafy pandafy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pandafy I found a minor issue while testing the widget in the credentials edit page, can you replicate it?

Yes, I am able to replicate it and it is occurring on master branch as well. I have opened #334 for it.

I believe it is unrelated to this PR.

@nemesifier

Copy link
Copy Markdown
Member Author

@pandafy I found a minor issue while testing the widget in the credentials edit page, can you replicate it?

Yes, I am able to replicate it and it is occurring on master branch as well. I have opened #334 for it.

I believe it is unrelated to this PR.

@pandafy you're right, so I think we can merge this one. Do you agree?

@nemesifier nemesifier left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

@pandafy shouldn't this line also be removed as part of this change?

<script>django._jsonSchemaWidgetUrl = "{% url schema_view_name %}";</script>

@pandafy pandafy left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@pandafy shouldn't this line also be removed as part of this change?

<script>django._jsonSchemaWidgetUrl = "{% url schema_view_name %}";</script>

Yes @nemesisdesign , you are right! 😄

We should than also remove this, otherwise test suite will fail.

def test_credentials_jsonschema_widget_presence(self):
url = reverse(f'admin:{self.app_label}_credentials_add')
schema_url = reverse(CredentialsSchemaWidget.schema_view_name)
expected = f'<script>django._jsonSchemaWidgetUrl = "{schema_url}";</script>'
self._login()
response = self.client.get(url)
self.assertContains(response, expected)

@nemesifier

Copy link
Copy Markdown
Member Author

@pandafy Thanks for your integration.

I have also noticed the advanced mode code in the JS refers to an ID, but the CSS paths should be converted to use classes.

pandafy added a commit to pandafy/openwisp-controller that referenced this pull request Dec 18, 2024
@nemesifier nemesifier closed this Jun 17, 2026
@nemesifier nemesifier deleted the improve-jsonwidget branch June 17, 2026 22:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants