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

chore: Update modifiers impl to use Node api and suppress Compose lint warnings on forked components #947

Merged
merged 9 commits into from
Feb 8, 2024

Conversation

soulcramer
Copy link
Contributor

📋 Changes

Make the following sparkUsageOverlay modifier to use the Modifier.Node api, the modifier minimumTouchTargetSize to use the material one as it has been made public and suppress the coming warnings from the next version of compose lint for components that we forked

🤔 Context

As said in #923 (comment) this PR aims to surface the changes required to upgrade to the next compose lint version

✅ Checklist

  • I have reviewed the submitted code.
  • I have tested on a phone device/emulator.

🗒️ Other info

Copy link
Contributor

github-actions bot commented Feb 7, 2024

🔣 PR title format

commit validation: failed!
please enter a commit message in the commitizen format.
commit "": "Update modifiers impl to use Node api and suppress Compose lint warnings on forked components"
pattern: (?s)(build|ci|docs|feat|fix|perf|refactor|style|test|chore|revert|bump)(\(\S+\))?!?:( [^\n\r]+)((\n\n.*)|(\s*))?$

Copy link
Contributor

github-actions bot commented Feb 7, 2024

🔣 PR title format

commit validation: failed!
please enter a commit message in the commitizen format.
commit "": "Update modifiers impl to use Node api and suppress Compose lint warnings on forked components"
pattern: (?s)(build|ci|docs|feat|fix|perf|refactor|style|test|chore|revert|bump)(\(\S+\))?!?:( [^\n\r]+)((\n\n.*)|(\s*))?$

@soulcramer soulcramer changed the title Update modifiers impl to use Node api and suppress Compose lint warnings on forked components chore: Update modifiers impl to use Node api and suppress Compose lint warnings on forked components Feb 7, 2024
Copy link
Contributor

github-actions bot commented Feb 7, 2024

🚨 UI regression detected! Checkout the paparazzi-delta artifact.
If these changes are expected, you can either:

  • manually run the gradlew recordPaparazziRelease and commit the new golden images
  • or ask @spark-ui-bot paparazzi golden images in this PR

Copy link
Contributor

github-actions bot commented Feb 7, 2024

Job Summary for Gradle

👷 Build → 🧑‍🔬 Test → 🕵️ Lint :: build-test-lint
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
spark-android assembleRelease 8.6 Build Scan NOT_PUBLISHED
spark-android globalCiUnitTest verifyPaparazziRelease 8.6 Build Scan NOT_PUBLISHED

Copy link
Contributor

github-actions bot commented Feb 7, 2024

Job Summary for Gradle

👷 Build → 🧑‍🔬 Test → 🕵️ Lint :: build-test-lint
Gradle Root Project Requested Tasks Gradle Version Build Outcome Build Scan®
spark-android assembleRelease 8.6 Build Scan NOT_PUBLISHED
spark-android lintRelease 8.6 Build Scan NOT_PUBLISHED
spark-android globalCiUnitTest verifyPaparazziRelease 8.6 Build Scan NOT_PUBLISHED

@@ -33,12 +33,12 @@
<plurals name="spark_rating_with_comments_a11y" tools:ignore="UnusedQuantity">
<item quantity="one">Note de %1$.1f pour %2$d avis</item>
<item quantity="other">Note de %1$.1f pour %2$d avis</item>
<item quantity="many">Note de %1$.1f pour %2$d avis</item>
<item quantity="many">Note de %1$.1f pour %2$d d’avis</item>
Copy link
Member

Choose a reason for hiding this comment

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

Are you sure about these additions of d' and de?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah I checked french plural rules and for the many category it should/must be present

soulcramer and others added 3 commits February 7, 2024 22:13
…Target.kt

Co-authored-by: Simon Marquis <simon.marquis@adevinta.com>
…Target.kt

Co-authored-by: Simon Marquis <simon.marquis@adevinta.com>
@soulcramer soulcramer merged commit 9d905fa into main Feb 8, 2024
5 checks passed
@soulcramer soulcramer deleted the update-code-for--lint branch February 8, 2024 09:27
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