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 Link to FAQ in the app #9164

Merged
merged 8 commits into from
Oct 26, 2022
Merged

Add Link to FAQ in the app #9164

merged 8 commits into from
Oct 26, 2022

Conversation

Yuuu2990
Copy link
Contributor

@Yuuu2990 Yuuu2990 commented Oct 21, 2022

What is it?

  • Bugfix (user facing)
  • Feature (user facing)
  • Codebase improvement (dev facing)
  • Meta improvement to the project (dev facing)

Description of the changes in your PR

According to the instruction of the maintainer, add link to FAQ in the app.

Before/After Screenshots/Screen Record

Before After
Before After

Fixes the following issue(s)

APK testing

The APK can be found by going to the "Checks" tab below the title. On the left pane, click on "CI", scroll down to "artifacts" and click "app" to download the zip file which contains the debug APK of this PR.

Due diligence

Update FAQ description

Co-authored-by: opusforlife2 <53176348+opusforlife2@users.noreply.github.com>
@Yuuu2990
Copy link
Contributor Author

@opusforlife2 I have committed your change. Hope everything is good. :-)

Copy link
Member

@AudricV AudricV left a comment

Choose a reason for hiding this comment

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

Thank you!

Please revert all the unneeded changes you or your IDE made, including the code style ones and the buttons size ones.

I think you tried to show everything without having to scroll, by reducing the buttons size. That's a really bad behavior, because:

  • you decrease the accessibility of these buttons;
  • devices have different aspect ratios, so you can't except to show everything on every device without having to scroll;
  • what is applicable on portrait is not always applicable on landscape (and that's the case here).

@AudricV AudricV added feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface labels Oct 23, 2022
@Yuuu2990
Copy link
Contributor Author

@AudricV @opusforlife2 Hi, according to the advice form AudricV, I show everything with having to scroll. It's my problem to ignore the problems what AudricV. Anyway, hope it's fine now. :-)

Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Thanks, looks almost good to me ;-)

app/src/main/res/layout/fragment_about.xml Outdated Show resolved Hide resolved
app/src/main/res/layout/fragment_about.xml Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
app/src/main/res/values/strings.xml Outdated Show resolved Hide resolved
Yuuu2990 and others added 4 commits October 26, 2022 14:24
Remove redundant id.

Co-authored-by: Stypox <stypox@pm.me>
Remove redundant id.

Co-authored-by: Stypox <stypox@pm.me>
Keep the uppercase for consistency.

Co-authored-by: Stypox <stypox@pm.me>
Modify the description of FAQ.

Co-authored-by: Stypox <stypox@pm.me>
@Yuuu2990
Copy link
Contributor Author

@Stypox @opusforlife2 Hi, according to the advice from you, I have committed the changes. You can check it. :-)

@AudricV AudricV removed their request for review October 26, 2022 06:36
Copy link
Member

@Stypox Stypox left a comment

Choose a reason for hiding this comment

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

Looks good to me, thank you!
I tested the changes myself since the CI has trouble using the NewPipeExtractor library I think.

@Stypox Stypox merged commit 1686060 into TeamNewPipe:dev Oct 26, 2022
@Yuuu2990
Copy link
Contributor Author

@Stypox @opusforlife2 Thank you very much for your suggestions while solving the issue. :-)

This was referenced Nov 4, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Issue is related to a feature in the app GUI Issue is related to the graphical user interface
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Link to FAQ in the app
4 participants