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

Use fieldset id instead of the fieldset title in Form when rendering the Field component #5921

Merged
merged 8 commits into from
Mar 26, 2024

Conversation

sneridagh
Copy link
Member

No description provided.

@sneridagh sneridagh requested a review from davisagli March 25, 2024 17:01
Copy link

netlify bot commented Mar 25, 2024

Deploy Preview for volto ready!

Name Link
🔨 Latest commit ed30884
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/6601ade6dc0d390008dab245
😎 Deploy Preview https://deploy-preview-5921--volto.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

Copy link

netlify bot commented Mar 25, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit ed30884
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/6601ade6a2b83500085c4237

Copy link

netlify bot commented Mar 25, 2024

Deploy Preview for plone-components canceled.

Name Link
🔨 Latest commit 7cb2687
🔍 Latest deploy log https://app.netlify.com/sites/plone-components/deploys/66029c4a8b626f0007cbbb48

Copy link

netlify bot commented Mar 25, 2024

Deploy Preview for volto canceled.

Name Link
🔨 Latest commit 7cb2687
🔍 Latest deploy log https://app.netlify.com/sites/volto/deploys/66029c4a9d7bf90008584caa

docs/source/upgrade-guide/index.md Outdated Show resolved Hide resolved
@@ -218,6 +218,14 @@ The `.stories.mdx` extension is no longer supported.
Although it is technically possible to keep the old version running, the script `volto-update-deps` will try to update to Storybook 8 every time you run it.
```

### Form component passes down the id of the current fieldset

There was a bug that was provoking to have `id` attributes with spaces on it, if the a fieldset was longer than one word.
Copy link
Collaborator

Choose a reason for hiding this comment

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

It's not clear to me what the symptom was, so how can I identify it?

Suggested change
There was a bug that was provoking to have `id` attributes with spaces on it, if the a fieldset was longer than one word.
A bug where a fieldset's `title` contains spaces would cause [INSERT DESCRIPTION OF SYMPTOM HERE] has been fixed.

Copy link
Member Author

Choose a reason for hiding this comment

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

@stevepiercy in order to fix this one, I ended up by retouching the whole thing again.
Please take a look again.

docs/source/upgrade-guide/index.md Outdated Show resolved Hide resolved
docs/source/upgrade-guide/index.md Outdated Show resolved Hide resolved
Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

I'm hella confused. This attempt to revise could make things better or worse. Please let me know.

docs/source/upgrade-guide/index.md Outdated Show resolved Hide resolved
docs/source/upgrade-guide/index.md Outdated Show resolved Hide resolved
docs/source/upgrade-guide/index.md Outdated Show resolved Hide resolved
@sneridagh
Copy link
Member Author

@stevepiercy yes, all is good now.

Copy link
Collaborator

@stevepiercy stevepiercy left a comment

Choose a reason for hiding this comment

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

Align changelog with docs.

One change, and I approve. Thank you!

packages/volto/news/5921.bugfix Outdated Show resolved Hide resolved
@sneridagh sneridagh merged commit ca64494 into main Mar 26, 2024
74 checks passed
@sneridagh sneridagh deleted the fieldsetnormalizedinfieldcomponents branch March 26, 2024 16:05
sneridagh added a commit that referenced this pull request Apr 16, 2024
* main: (21 commits)
  Use fieldset `id` instead of the fieldset `title` in Form when rendering the Field component (#5921)
  Fix registry wrong default primitive type (#5925)
  Release @plone/types 1.0.0-alpha.8
  Improve types for APIExpanders, export all (#5918)
  Missing step in storybook migration (#5913)
  Push missing .d.ts files
  Release 18.0.0-alpha.25
  Release generate-volto 9.0.0-alpha.14
  Update Docs: Pastanaga UI and Quanta (#5903)
  Upgrade to StoryBook 8 (#5912)
  Fix Storybook in app gen for Volto 18 (#5911)
  Release 18.0.0-alpha.24
  Upgrade `@typescript-eslint` (#5910)
  Add new cypress helper for slate `getSlateEditorAndType` (#5909)
  Add id attributes to discussions container and individual comments (#5905)
  Move testing-library cypress commands import to inner commands so it can be imported (#5906)
  Release 18.0.0-alpha.23
  Release @plone/registry 1.5.4
  Improve the usage of RAZZLE_JEST_CONFIG (#5901)
  fix reordering of slots function (#5840)
  ...
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