Skip to content

fix: spinner hint display #2672

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

Merged
merged 7 commits into from
Apr 28, 2025

Conversation

rahul31124
Copy link
Collaborator

@rahul31124 rahul31124 commented Apr 23, 2025

Fixes #2188

Changes

Changed layout from LinearLayout to ConstraintLayout.

Screenshots / Recordings

Before
Config_Before
After
Config_After

Checklist:

  • No hard coding: I have used resources from strings.xml, dimens.xml and colors.xml without hard coding any value.
  • No end of file edits: No modifications done at end of resource files strings.xml, dimens.xml or colors.xml.
  • Code reformatting: I have reformatted code and fixed indentation in every file included in this pull request.
  • No extra space: My code does not contain any extra lines or extra spaces than the ones that are necessary.

Summary by Sourcery

Refactor the time interval layout in the create config activity from LinearLayout to ConstraintLayout to improve UI positioning and responsiveness

Bug Fixes:

  • Fixed potential layout alignment issues by using ConstraintLayout's more advanced positioning capabilities

Enhancements:

  • Migrated the time interval input section from LinearLayout to ConstraintLayout for more precise and flexible UI constraints

Copy link

sourcery-ai bot commented Apr 23, 2025

Reviewer's Guide by Sourcery

This pull request replaces the LinearLayout with ConstraintLayout in activity_create_config.xml to fix the spinner hint display issue. Constraints and weights have been added to properly align the TextView, EditText, and Spinner.

No diagrams generated as the changes look simple and do not need a visual representation.

File-Level Changes

Change Details Files
Replaced LinearLayout with ConstraintLayout to fix spinner hint display issue.
  • Changed the root layout from LinearLayout to ConstraintLayout.
  • Added constraints for the TextView, EditText, and Spinner to properly align them within the ConstraintLayout.
  • Removed layout_gravity and layout_weight attributes.
  • Added weights to the EditText and Spinner to manage horizontal space distribution.
app/src/main/res/layout/activity_create_config.xml

Assessment against linked issues

Issue Objective Addressed Explanation
#2188 Display the time interval units to the user when generating a config file.

Tips and commands

Interacting with Sourcery

  • Trigger a new review: Comment @sourcery-ai review on the pull request.
  • Continue discussions: Reply directly to Sourcery's review comments.
  • Generate a GitHub issue from a review comment: Ask Sourcery to create an
    issue from a review comment by replying to it. You can also reply to a
    review comment with @sourcery-ai issue to create an issue from it.
  • Generate a pull request title: Write @sourcery-ai anywhere in the pull
    request title to generate a title at any time. You can also comment
    @sourcery-ai title on the pull request to (re-)generate the title at any time.
  • Generate a pull request summary: Write @sourcery-ai summary anywhere in
    the pull request body to generate a PR summary at any time exactly where you
    want it. You can also comment @sourcery-ai summary on the pull request to
    (re-)generate the summary at any time.
  • Generate reviewer's guide: Comment @sourcery-ai guide on the pull
    request to (re-)generate the reviewer's guide at any time.
  • Resolve all Sourcery comments: Comment @sourcery-ai resolve on the
    pull request to resolve all Sourcery comments. Useful if you've already
    addressed all the comments and don't want to see them anymore.
  • Dismiss all Sourcery reviews: Comment @sourcery-ai dismiss on the pull
    request to dismiss all existing Sourcery reviews. Especially useful if you
    want to start fresh with a new review - don't forget to comment
    @sourcery-ai review to trigger a new review!

Customizing Your Experience

Access your dashboard to:

  • Enable or disable review features such as the Sourcery-generated pull request
    summary, the reviewer's guide, and others.
  • Change the review language.
  • Add, remove or edit custom review instructions.
  • Adjust other review settings.

Getting Help

Copy link

@sourcery-ai sourcery-ai bot left a comment

Choose a reason for hiding this comment

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

Hey @rahul31124 - I've reviewed your changes - here's some feedback:

Overall Comments:

  • It's great you included before and after screenshots to show the impact of your changes.
Here's what I looked at during the review
  • 🟡 General issues: 1 issue found
  • 🟢 Security: all looks good
  • 🟢 Testing: all looks good
  • 🟢 Complexity: all looks good
  • 🟢 Documentation: all looks good

Sourcery is free for open source - if you like our reviews please consider sharing them ✨
Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.

android:layout_width="match_parent"
android:layout_height="wrap_content"
android:layout_marginStart="@dimen/create_config_margin1"
android:orientation="horizontal">
android:layout_marginStart="@dimen/create_config_margin1">

<TextView
android:id="@+id/time_interval_title"
style="@style/Base.TextAppearance.AppCompat.Title"
android:layout_width="wrap_content"
Copy link

Choose a reason for hiding this comment

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

suggestion: Spinner width may need to be set to 0dp for chain weights to apply.

The spinner has a horizontal weight assigned, yet its width is set to wrap_content. In a ConstraintLayout chain with weighted distribution, each view that participates should use a 0dp width. Consider revising the spinner’s width to 0dp to ensure its weight is honored as intended.

Suggested implementation:

    <Spinner
        android:id="@+id/time_interval_spinner"
        android:layout_width="0dp"
        android:layout_height="wrap_content"
        app:layout_constraintHorizontal_weight="1"
        ... />

Ensure that the Spinner element has its associated constraints in the ConstraintLayout chain correctly set so that the 0dp width together with the assigned weight produces the desired layout.
If your spinner is defined elsewhere or uses a different id or attributes, adjust the id/attributes accordingly.

Copy link

github-actions bot commented Apr 23, 2025

@marcnause marcnause force-pushed the fix-spinner-hint-display branch from dc1239e to f687022 Compare April 28, 2025 17:00
Copy link
Contributor

@marcnause marcnause left a comment

Choose a reason for hiding this comment

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

Successfully tested on my device,

@rahul31124 Thank you for your contribution!

@marcnause marcnause merged commit ca724dd into fossasia:development Apr 28, 2025
3 checks passed
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.

Time interval units are not shown to the user.
2 participants