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

Remove label and placeholder from context and form, add optional provideDefaultLabelFromFieldName property to bridges. #1248

Merged
merged 15 commits into from
Jun 5, 2023

Conversation

Monteth
Copy link
Member

@Monteth Monteth commented Mar 17, 2023

Changes are described here: #973 (comment).

This version doesn't include the provideDefaultLabelFromFieldName bridge property in SimpleSchema, since it always delivers a 'label' property. We cannot distinguish whether it was inferred from the field name or entered manually into the schema.

@github-actions github-actions bot added Area: Bridge Affects some of the bridge packages Area: Core Affects the uniforms package Area: Docs Affects the documentation or reproductions seed Area: Theme Affects some of the theme packages Bridge: GraphQL Affects the uniforms-bridge-graphql package Bridge: JSON Schema Affects the uniforms-bridge-json-schema package Bridge: Zod Affects the uniforms-bridge-zod package Theme: AntD Affects the uniforms-antd package Theme: Bootstrap 3 Affects the uniforms-bootstrap3 package Theme: Bootstrap 4 Affects the uniforms-bootstrap4 package Theme: Bootstrap 5 Affects the uniforms-bootstrap5 package Theme: Material-UI Affects the uniforms-material package Theme: MUI Affects the uniforms-mui package Theme: Semantic UI Affects the uniforms-semantic package Theme: Unstyled Affects the uniforms-unstyled package labels Mar 17, 2023
@Monteth
Copy link
Member Author

Monteth commented Mar 20, 2023

I run into a problem implementing the provideDefaultLabelFromFieldName functionality in SimpleSchema, you can follow the issue here: longshotlabs/simpl-schema#480

Copy link
Contributor

@radekmie radekmie left a comment

Choose a reason for hiding this comment

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

As I understand, as soon as we migrate to Zod in tests or Simple Schema solves the issue with the labels, we could remove all of these label: '' from tests?

packages/uniforms-bridge-graphql/src/GraphQLBridge.ts Outdated Show resolved Hide resolved
packages/uniforms-bridge-zod/src/ZodBridge.ts Outdated Show resolved Hide resolved
packages/uniforms/src/BaseForm.tsx Outdated Show resolved Hide resolved
packages/uniforms/src/useField.tsx Outdated Show resolved Hide resolved
Conflicts:
	packages/uniforms-semantic/__tests__/LongTextField.tsx
	packages/uniforms-semantic/__tests__/NumField.tsx
@Monteth
Copy link
Member Author

Monteth commented Apr 17, 2023

As I understand, as soon as we migrate to Zod in tests or Simple Schema solves the issue with the labels, we could remove all of these label: '' from tests?

Correct. Since we get rid of label in context, we can no longer tell SimpleSchema not to derive the label from the field name, so we have to pass it explicitly. This behavior is specific to SimpleSchema only.

@codecov
Copy link

codecov bot commented Apr 28, 2023

Codecov Report

Merging #1248 (8d605b2) into master (cfaf81a) will decrease coverage by 0.97%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master    #1248      +/-   ##
==========================================
- Coverage   97.48%   96.51%   -0.97%     
==========================================
  Files         231      231              
  Lines        3821     3821              
  Branches     1031     1029       -2     
==========================================
- Hits         3725     3688      -37     
- Misses          4       12       +8     
- Partials       92      121      +29     
Impacted Files Coverage Δ
packages/uniforms/src/BaseForm.tsx 100.00% <ø> (ø)
packages/uniforms/src/connectField.tsx 96.55% <ø> (-3.45%) ⬇️
packages/uniforms/src/filterDOMProps.ts 100.00% <ø> (ø)
...kages/uniforms-bridge-graphql/src/GraphQLBridge.ts 97.82% <100.00%> (+0.09%) ⬆️
...niforms-bridge-json-schema/src/JSONSchemaBridge.ts 100.00% <100.00%> (ø)
...-bridge-simple-schema-2/src/SimpleSchema2Bridge.ts 97.82% <100.00%> (ø)
...rms-bridge-simple-schema/src/SimpleSchemaBridge.ts 100.00% <100.00%> (ø)
packages/uniforms-bridge-zod/src/ZodBridge.ts 100.00% <100.00%> (ø)
packages/uniforms/src/useField.tsx 100.00% <100.00%> (ø)

... and 17 files with indirect coverage changes

@github-actions github-actions bot added Bridge: SimpleSchema Affects the uniforms-bridge-simple-schema package Bridge: SimpleSchema (v2) Affects the uniforms-bridge-simple-schema-2 package labels Apr 28, 2023
@Monteth Monteth marked this pull request as ready for review May 23, 2023 14:31
@Monteth Monteth requested a review from wadamek65 as a code owner May 23, 2023 14:31
@Monteth Monteth requested a review from radekmie May 26, 2023 10:58
radekmie
radekmie previously approved these changes Jun 5, 2023
kestarumper
kestarumper previously approved these changes Jun 5, 2023
@Monteth Monteth dismissed stale reviews from kestarumper and radekmie via 8d605b2 June 5, 2023 11:33
@radekmie radekmie merged commit 1c69877 into master Jun 5, 2023
5 of 6 checks passed
@radekmie radekmie deleted the 973-remove-label-and-placeholder branch June 5, 2023 12:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Area: Bridge Affects some of the bridge packages Area: Core Affects the uniforms package Area: Docs Affects the documentation or reproductions seed Area: Theme Affects some of the theme packages Bridge: GraphQL Affects the uniforms-bridge-graphql package Bridge: JSON Schema Affects the uniforms-bridge-json-schema package Bridge: SimpleSchema (v2) Affects the uniforms-bridge-simple-schema-2 package Bridge: SimpleSchema Affects the uniforms-bridge-simple-schema package Bridge: Zod Affects the uniforms-bridge-zod package Theme: AntD Affects the uniforms-antd package Theme: Bootstrap 3 Affects the uniforms-bootstrap3 package Theme: Bootstrap 4 Affects the uniforms-bootstrap4 package Theme: Bootstrap 5 Affects the uniforms-bootstrap5 package Theme: Material-UI Affects the uniforms-material package Theme: MUI Affects the uniforms-mui package Theme: Semantic UI Affects the uniforms-semantic package Theme: Unstyled Affects the uniforms-unstyled package
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

None yet

3 participants