Skip to content

Task/standardize command bar directions #355

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

Open
wants to merge 6 commits into
base: main
Choose a base branch
from

Conversation

janiceferns02
Copy link
Contributor

Contributor checklist


Description

  • Added : after the SOURCE_ISO -> TARGET_ISO part of the translate placeholder

  • To Fix the horizontal distance between the command bar cursor character and the rest of the text

  1. Removed command bar cursor character implementation using Button
  2. Replaced with EditText having custom cursor drawable
  3. Updated XML to use EditText inside command bar layout in both portrait and landscape xml files

Testing

  1. Verified Placeholder should show in format: SOURCE_ISO -> TARGET_ISO:
  2. Verified Cursor should always be visible and should move properly after each typed character
  3. Verified UI Alignment in both portrait and landscape modes
  4. Verified the changes in light and dark modes

Related issue

Copy link

github-actions bot commented Apr 7, 2025

Thank you for the pull request! ❤️

The Scribe-Android team will do our best to address your contribution as soon as we can. If you're not already a member of our public Matrix community, please consider joining! We'd suggest using Element as your Matrix client, and definitely join the General and Android rooms once you're in. Also consider attending our bi-weekly Saturday dev syncs. It'd be great to meet you 😊

Copy link

github-actions bot commented Apr 7, 2025

Maintainer Checklist

The following is a checklist for maintainers to make sure this process goes as well as possible. Feel free to address the points below yourself in further commits if you realize that actions are needed :)

  • The linting and formatting workflows within the PR checks do not indicate new errors in the files changed

  • The CHANGELOG has been updated with a description of the changes for the upcoming release and the corresponding issue (if necessary)

@andrewtavis
Copy link
Member

Thanks for this, @janiceferns02! We'll try to get to the review in the coming days :)

@andrewtavis andrewtavis self-requested a review April 11, 2025 15:13
@andrewtavis
Copy link
Member

All's looking good here, @janiceferns02! Really appreciate your initiative here 😊

Two minor comments based on the image below:

Screenshot_20250411_173332

  • You you remove the grey vertical bar from the prompts so that we only have the blue one?
  • Could you remove the blinking for the blue cursor? This was something that we decided with the original designer a while ago, that it doesn't make sense to have the cursor blink as the normal cursor is also still blinking. As far as UI appeal, having the cursor blink out of sync with the normal one for the text proxy is also something that we can't control, so the decision was to just keep it solid.

Please let us know if you have any questions on the comments above!

@janiceferns02
Copy link
Contributor Author

All's looking good here, @janiceferns02! Really appreciate your initiative here 😊

Two minor comments based on the image below:

Screenshot_20250411_173332

  • You you remove the grey vertical bar from the prompts so that we only have the blue one?
  • Could you remove the blinking for the blue cursor? This was something that we decided with the original designer a while ago, that it doesn't make sense to have the cursor blink as the normal cursor is also still blinking. As far as UI appeal, having the cursor blink out of sync with the normal one for the text proxy is also something that we can't control, so the decision was to just keep it solid.

Please let us know if you have any questions on the comments above!

Understood, I will make the changes.

@andrewtavis
Copy link
Member

Thanks much, @janiceferns02! Let us know when it's ready for another review 😊

@andrewtavis
Copy link
Member

Quick check in here, @janiceferns02 👋 Do you need any support with the changes for this PR :) We're happy to help as needed! 😊

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