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

Add Installation Instructions #140

Merged
merged 7 commits into from
Sep 24, 2024

Conversation

Linfye
Copy link
Collaborator

@Linfye Linfye commented Sep 22, 2024

Contributor checklist


Description

The current page looks like this:
image
And I have 3 questions:

  1. Are there any click events that need to be added to this page? Currently, my PR has turned this page into a static page.
  2. Currently, there is a 'Scribe' title in the top-left corner of the page, which is not present in the Figma design. I'm not sure how to remove it, could you give me some tips?
  3. In the Figma design, "Open Scribe Settings" has an underline, and I don't know how to add it in the XML file. Could you help me with that?

Thanks. :)

Related issue

Copy link

github-actions bot commented Sep 22, 2024

Thank you for the pull request!

The Scribe team will do our best to address your contribution as soon as we can. 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 :)

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 joining our bi-weekly Saturday dev syncs. It'd be great to have you!

Maintainer checklist

  • The commit messages for the remote branch should be checked to make sure the contributor's email is set up correctly so that they receive credit for their contribution

    • The contributor's name and icon in remote commits should be the same as what appears in the PR
    • If there's a mismatch, the contributor needs to make sure that the email they use for GitHub matches what they have for git config user.email in their local Scribe-Android repo
  • 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 andrewtavis self-requested a review September 22, 2024 18:36
@andrewtavis
Copy link
Member

andrewtavis commented Sep 22, 2024

Hey @Linfye! This is looking amazing 🤩 Really can't wait to bring this in as the app menu will then be at a great point :)

A couple comments on some things that need to change before we merge:

The texts for the field should be:

  • app.installation.keyboard.keyboard_settings
  • app.installation.keyboard.text_2 (we don't need test_1 for Android)
  • app.installation.keyboard.text_3

So the final text should be:

1. Open keyboard settings
2. Activate keyboards that you want to use
3. When typing, press [KEYBOARD_ICON] to select keyboards

Specifically we don't want 🌐, but rather ⌨️ and the icon that's used on Android for switching keyboard input methods. On iOS we have the globe for this, but for Android there's a keyboard icon.

Answers to your questions:

  1. Are there any click events that need to be added to this page? Currently, my PR has turned this page into a static page.
  • I think all's fine. We can add some toasts for the various buttons, but aside from that all's fine :)
  1. Currently, there is a 'Scribe' title in the top-left corner of the page, which is not present in the Figma design. I'm not sure how to remove it, could you give me some tips?
  1. In the Figma design, "Open Scribe Settings" has an underline, and I don't know how to add it in the XML file. Could you help me with that?
  • I'm not too sure on this, but let's not worry about it too much as the XML file will be changing in the comings months when we start converting the app layouts to jetpack compose. No need to worry for now :)

@angrezichatterbox
Copy link
Member

3. Currently, there is a 'Scribe' title in the top-left corner of the page, which is not present in the Figma design. I'm not sure how to remove it, could you give me some tips?

A possible solution would be to hide the action bar on Creation of the MainActivity using supportActionBar.hide() and view pager changes would show the action bar and hide according to which page the user would be. There might be additional changes to be made but this could work.

@Linfye
Copy link
Collaborator Author

Linfye commented Sep 23, 2024

Thank you for your responses. I have made the changes. Could you review it again? 😊

@andrewtavis
Copy link
Member

Sorry to ask for two final changes, @Linfye :)

We also want the keyboard installation field to be a button, which is why Open keyboard settings is blue and eventually underlined. Could you add an action to that field such that when you press it it opens the list of keyboards to install? So when we click anywhere on the keyboard installation instructions we should go to this page of the settings:

Screenshot_20240923_081747

And the last thing - also sorry for not being clear with this - can we get the ⌨️ changed to the icon that Android uses for switching keyboard inputs? Specifically the one below:

Screenshot 2024-09-23 at 08 18 41

Really is looking great! 😊 So nice to see Scribe-Android like this finally :)

@Linfye
Copy link
Collaborator Author

Linfye commented Sep 23, 2024

I've made those changes you mentioned. You can review again now. Thanks. 😊

@angrezichatterbox
Copy link
Member

angrezichatterbox commented Sep 23, 2024

@Linfye Could u show the action bar in case of the other pages using supportActionBar?.show() command when the view pager changes it position. If it causes adjustment issues in case of nav bar changing position you could try modifying the xml layout of main activity to fill system window.

Thank you for the efforts you have been adding to the project so far :)

@Linfye
Copy link
Collaborator Author

Linfye commented Sep 23, 2024

@angrezichatterbox, I'm not quite clear about what you mean. Are you suggesting to turn on the supportActionBar on the fragment_main.xml? Could you explain in detail how to do it? 😊

@angrezichatterbox
Copy link
Member

angrezichatterbox commented Sep 23, 2024

@angrezichatterbox, I'm not quite clear about what you mean. Are you suggesting to turn on the supportActionBar on the fragment_main.xml? Could you explain in detail how to do it? 😊

Sorry I could have been more clear.

So currently you have hidden the supportActionBar on the MainFragment. The action bar is not visible for the other pages as well. A possible fix for this would be to hide the support Action bar onCreate in the MainActivity.

and then in the Main Activity in the view pager modify it to show the action bar for the other pages.

bottomNavigationView.setOnNavigationItemSelectedListener { menuItem ->
when (menuItem.itemId) {
R.id.installation -> {
viewPager.setCurrentItem(0, true)
binding.fragmentContainer.visibility = View.GONE
setActionBarTitle(R.string.app_launcher_name)
true
}
R.id.info -> {
viewPager.setCurrentItem(2, true)
binding.fragmentContainer.visibility = View.GONE
setActionBarTitle(R.string.app_about_title)
true
}
R.id.settings -> {
viewPager.setCurrentItem(1, true)
binding.fragmentContainer.visibility = View.GONE
setActionBarTitle(R.string.app_settings_title)
true
}
else -> {
false
}
}
}
}

This might cause the nav bar something to move up and down as the action bar is being changed continuously a possible fix for this would be to fill in window size for the relative layout of the main activity and then accordingly set the background color as well.

If you have any more doubts Feel free to ask.

@Linfye
Copy link
Collaborator Author

Linfye commented Sep 24, 2024

I tried several methods, but the action bar always has a strange up and down issue. In the end, I changed the layout of the Installation page and removed the title "Scribe" from the strings.xml characters. Currently, the page looks like this.

image

And action bar from other pages can display as expected.

@andrewtavis
Copy link
Member

I'll let @angrezichatterbox speak to the action bar, @Linfye :) For the icon I was hoping that Android had a pre-built one we could use, but it looks like there's not one. I went ahead and made it :) Can you use the SVGs I'm attaching for light mode and dark mode?

android_keyboard_icon_dark
android_keyboard_icon_light

Thanks so much!

@Linfye
Copy link
Collaborator Author

Linfye commented Sep 24, 2024

I'll let @angrezichatterbox speak to the action bar, @Linfye :) For the icon I was hoping that Android had a pre-built one we could use, but it looks like there's not one. I went ahead and made it :) Can you use the SVGs I'm attaching for light mode and dark mode?

android_keyboard_icon_dark android_keyboard_icon_light

Thanks so much!

OK, done. Thanks for the images! 😊

@andrewtavis
Copy link
Member

Let us know what the final review on your end is, @angrezichatterbox!

Copy link
Member

@angrezichatterbox angrezichatterbox left a comment

Choose a reason for hiding this comment

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

The PR looks awesome @Linfye. The action bar works fine and is present for all pages.

@andrewtavis
Copy link
Member

Will play around with it a bit more after work and send along the final review, @Linfye! Thanks :)

Copy link
Member

@andrewtavis andrewtavis left a comment

Choose a reason for hiding this comment

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

All's looking really good, @Linfye! I'll bring this in and note a few more things that need to be done in a new issue 😊 So awesome to see Scribe-Android like this :D

@andrewtavis andrewtavis merged commit 11b5b44 into scribe-org:main Sep 24, 2024
1 check passed
@Linfye Linfye deleted the installation-screen branch September 25, 2024 02:30
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.

3 participants