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

Fix(web-twig): Do not render input props when set to false #1117

Merged
merged 2 commits into from
Nov 30, 2023

Conversation

literat
Copy link
Collaborator

@literat literat commented Oct 25, 2023

Description

The input attribute will render even if there are set to false. This should solve the problem, but I do not know if it affects anything else. The tests are ok. But you know.

Additional context

Issue reference

https://almamedia.slack.com/archives/C0502FGLG3G/p1698159019944579


Before submitting the PR, please make sure you do the following

  • Read the Contributing Guidelines.
  • Follow the PR Title/Commit Message Convention.
  • Provide a description in this PR that addresses what the PR is solving, or reference the issue that it solves (e.g. fixes #123).
  • Ideally, include relevant tests that fail without this PR but pass with it.

@netlify
Copy link

netlify bot commented Oct 25, 2023

Deploy Preview for spirit-design-system-demo canceled.

Name Link
🔨 Latest commit f731f7e
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-demo/deploys/65688c40a99577000813893a

@github-actions github-actions bot added the bug Something isn't working label Oct 25, 2023
@netlify
Copy link

netlify bot commented Oct 25, 2023

Deploy Preview for spirit-design-system-storybook canceled.

Name Link
🔨 Latest commit f731f7e
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-storybook/deploys/65688c40ae19210008ef5410

@literat literat force-pushed the fix/web-twig-render-input-props-false branch from 30dba34 to 54b6850 Compare October 25, 2023 04:52
@netlify
Copy link

netlify bot commented Oct 25, 2023

Deploy Preview for spirit-design-system-validations canceled.

Name Link
🔨 Latest commit f731f7e
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-validations/deploys/65688c40d210900008bcf71f

@nx-cloud
Copy link

nx-cloud bot commented Oct 25, 2023

☁️ Nx Cloud Report

CI is running/has finished running commands for commit 54b6850. As they complete they will appear below. Click to see the status, the terminal output, and the build insights.

📂 See all runs for this CI Pipeline Execution


✅ Successfully ran 3 targets

Sent with 💌 from NxCloud.

@netlify
Copy link

netlify bot commented Oct 25, 2023

Deploy Preview for spirit-design-system-react canceled.

Name Link
🔨 Latest commit f731f7e
🔍 Latest deploy log https://app.netlify.com/sites/spirit-design-system-react/deploys/65688c40511791000843f546

@coveralls
Copy link

coveralls commented Oct 25, 2023

Coverage Status

coverage: 96.46%. remained the same
when pulling f731f7e on fix/web-twig-render-input-props-false
into 4adbc16 on main.

Copy link
Contributor

@dlouhak dlouhak left a comment

Choose a reason for hiding this comment

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

Btw, we're dealing with the same problem in the ModalDialog and novalidate prop. That prop is applied via mainProps(), but I'm afraid we're not able to apply the same condition there because some attributes may required false value. 🙈

@literat
Copy link
Collaborator Author

literat commented Nov 23, 2023

Btw, we're dealing with the same problem in the ModalDialog and novalidate prop. That prop is applied via mainProps(), but I'm afraid we're not able to apply the same condition there because some attributes may required false value. 🙈

I will check all those places and rewrite the documentation thus it is a native attribute and must be treated that way.

@literat literat force-pushed the fix/web-twig-render-input-props-false branch from 54b6850 to 0f8d6cb Compare November 29, 2023 14:04
@literat
Copy link
Collaborator Author

literat commented Nov 29, 2023

@dlouhak @adamkudrna Please, give this a second try. I have unified the usage of space between braces and also tried to better describe the usage of "void" attributes in our docs.

@literat literat force-pushed the fix/web-twig-render-input-props-false branch from e11041f to f731f7e Compare November 30, 2023 13:21
@literat literat merged commit 40b99a0 into main Nov 30, 2023
35 checks passed
@literat literat deleted the fix/web-twig-render-input-props-false branch November 30, 2023 13:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants