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 option to hide clock #146

Closed
wants to merge 2 commits into from

Conversation

CubeOfCheese
Copy link

@CubeOfCheese CubeOfCheese commented Aug 10, 2023

Addresses #81

Adds toggle control in options menu to display/hide the clock and date.

I am pretty new to Android development so all suggestions and advice are welcome.

Copy link

@hayribakici hayribakici left a comment

Choose a reason for hiding this comment

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

Good job, please consider the review comments.

val isClockDisplayed = settings.getBoolean(getString(R.string.prefs_settings_key_toggle_clock), true)

if (!isClockDisplayed) {
home_fragment_time.text = ""

Choose a reason for hiding this comment

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

Instead of setting the clock and date to an empty string, consider using visibility. In your case it would be

home_fragment_time.visibility = View.INVISIBLE
home_fragmen_date.visibility = View.INVISIBLE

and analogously with View.VISIBLE when the time and date shold be displayed.

Copy link
Author

Choose a reason for hiding this comment

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

I did try this but it didn't seem to work. I will try to investigate further. I agree visibility is the ideal way to handle this.

Copy link
Author

Choose a reason for hiding this comment

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

Even if I set the visisbility to invisible directly in the xml, the clock still displays. So I'm thinking something else is interfering

Choose a reason for hiding this comment

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

@CubeOfCheese Any luck with this issue?

Copy link

Choose a reason for hiding this comment

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

I tried to set the visibility and the alpha value as well but it gets reset somehow. Did not find out where exactly.

What works is to set the height to zero:

home_fragment_time.height=0
home_fragment_date.height=0

val settings = requireContext().getSharedPreferences(getString(R.string.prefs_settings), Context.MODE_PRIVATE)
val isClockDisplayed = settings.getBoolean(getString(R.string.prefs_settings_key_toggle_clock), false)
if (isClockDisplayed) {
home_fragment_time.setOnClickListener {
Copy link

@hayribakici hayribakici Aug 10, 2023

Choose a reason for hiding this comment

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

When setting to INVISIBLE (see above), you don't need to check for isClockDisplayed, since the time and date are not visible and therefore the users are unable to interact with it (e.g. clicking on it). You can remove the condition.

options_fragment_toggle_clock.isChecked = isClockDisplayed

options_fragment_toggle_clock.setOnCheckedChangeListener { _, checked ->
val settings = requireContext().getSharedPreferences(getString(R.string.prefs_settings), MODE_PRIVATE)

Choose a reason for hiding this comment

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

You already have an instance of settings in line 66. You don't need it here anymore.

@CubeOfCheese
Copy link
Author

Oh also I do not have translations in the strings.xml files. How would I go about doing that?

@hayribakici
Copy link

hayribakici commented Aug 11, 2023

@CubeOfCheese I wouldn't worry about it, right now 😄.

@hayribakici
Copy link

@CubeOfCheese as for future pull requests (in any repository), you can resize your screenshots (eg. to 300 px) with the img tag like this:

<img src="<your url here>" height="300px" />

Copy link
Collaborator

@TobiTenno TobiTenno left a comment

Choose a reason for hiding this comment

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

@CubeOfCheese these look awesome. please revise based on some of the other review comments (like duplicate vals), and i'll get this another review shortly!

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.

4 participants