Skip to content
This repository has been archived by the owner on Nov 1, 2022. It is now read-only.

For #12151 - Add support for empty step value in TimePicker #12802

Merged
merged 2 commits into from
Sep 22, 2022

Conversation

Alexandru2909
Copy link
Contributor

@Alexandru2909 Alexandru2909 commented Sep 19, 2022

Fixes Fenix #26943.

Pull Request checklist

  • Quality: This PR builds and passes detekt/ktlint checks (A pre-push hook is recommended)
  • Tests: This PR includes thorough tests or an explanation of why it does not
  • Changelog: This PR includes a changelog entry or does not need one
  • Accessibility: The code in this PR follows accessibility best practices or does not include any user facing features

After merge

  • Milestone: Make sure issues closed by this pull request are added to the milestone of the version currently in development.
  • Breaking Changes: If this is a breaking change, please push a draft PR on Reference Browser to address the breaking issues.

GitHub Automation

Fixes #12151

@rocketsroger
Copy link
Contributor

Let's wait till the branch is cut before landing this. Once this is soaked in Nightly and we see the crash is fixed, we can lift it to beta. Thanks

@rocketsroger rocketsroger added the do not land PRs that requires coordination before landing label Sep 20, 2022
@mergify
Copy link
Contributor

mergify bot commented Sep 20, 2022

This pull request has conflicts when rebasing. Could you fix it @Alexandru2909? 🙏

@Alexandru2909 Alexandru2909 force-pushed the 12151_1 branch 2 times, most recently from be3f989 to f5de652 Compare September 21, 2022 06:08
Copy link
Contributor

@Amejia481 Amejia481 left a comment

Choose a reason for hiding this comment

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

Thanks for the patch, LGTM!
Let's try to add formatting only to the areas that has been changed, as adding formatting to files makes reviewing a bit hard, as it's not straightforward what have been changed 😄.

@Amejia481 Amejia481 removed the request for review from csadilek September 21, 2022 13:29
@mergify
Copy link
Contributor

mergify bot commented Sep 21, 2022

This pull request has conflicts when rebasing. Could you fix it @Alexandru2909? 🙏

@Alexandru2909
Copy link
Contributor Author

Thanks for the patch, LGTM! Let's try to add formatting only to the areas that has been changed, as adding formatting to files makes reviewing a bit hard, as it's not straightforward what have been changed 😄.

I tried formatting only the lines I changed but for me ktlint fails locally for the whole file, so I think we need to add the formatting for both GeckoPromptDelegateTest.kt and GeckoPromptDelegate.kt in this PR. I separated the formatting in another commit so it's easier to follow and review.

@rocketsroger rocketsroger removed the do not land PRs that requires coordination before landing label Sep 21, 2022
@Alexandru2909 Alexandru2909 added the 🛬 needs landing PRs that are ready to land label Sep 22, 2022
@mergify mergify bot merged commit af7078f into mozilla-mobile:main Sep 22, 2022
@LaurentiuApahideanSV
Copy link

I tested the issue on Fenix Nightly 107.0a1 and the issue no longer occurs. Seconds and milliseconds can be selected without crashing from the example provided here: #12632.

Device used:

  • Samsung Galaxy S22 Ultra (Android 12)
  • Huawei MediaPad M3 (Android 7)

@rocketsroger
Copy link
Contributor

@Mergifyio backport releases_v106.0.0

@mergify
Copy link
Contributor

mergify bot commented Sep 23, 2022

backport releases_v106.0.0

❌ No backport have been created

  • Backport to branch releases_v106.0.0 failed: Branch not found

@rocketsroger
Copy link
Contributor

@Mergifyio backport releases/106.0

@mergify
Copy link
Contributor

mergify bot commented Sep 23, 2022

backport releases/106.0

✅ Backports have been created

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
🛬 needs landing PRs that are ready to land
Projects
None yet
Development

Successfully merging this pull request may close these issues.

NumberFormatException: empty String Support step attribute in <input type="time"> etc
4 participants