Skip to content

feat: Display compose button when viewing hashtags #254

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

Conversation

sanao1006
Copy link
Contributor

Overview

The previous code didn't display the compose button when viewing hashtags, which was inconvenient.

Therefore, display the button when viewing hashtags.

Changes

  • display the button when viewing hashtags.
  • insert the hashtag in the draft message when clicking the button

Video

Screen_recording_20231115_154202.webm
Screen_recording_20231115_154336.webm

Concern

  • I didn't know how actionButtonPresent() should be implemented. It seems to be working fine, but maybe I should implement

Related issue

Fixes #228

The previous code didn't display the compose button when viewing hashtags, which was inconvenient.

Therefore, display the button when viewing hashtags.

Additionally, insert the hashtag in the draft message when clicking the button

Fixes pachli#228
@sanao1006 sanao1006 marked this pull request as ready for review November 15, 2023 07:16
In the previous code, a hashtag string passed from StatusListActivity to ComposeActivity using Intent was displayed on the screen by modifying the codes in ComposeActivity.

This reduces scalability.

Therefore, Display the hashtag text on the screen without modifying `ComposeActivity` by using static `startIntent()`.

Fixes pachli#254, pachli#228
@sanao1006 sanao1006 requested a review from nikclayton November 16, 2023 01:47
@nikclayton
Copy link
Contributor

That's looking great, thanks.

I've been trying this out, and one thing that occurred to me is -- where should the cursor be when the user first creates the post?

At the moment it's placed at the end of the hashtag, and there's no space between the cursor and the tag. So the user is either going to have to insert a space every time, or manually move the cursor back to the start of the tag if they want the tag to be at the end of the post.

I suspect it would be better if the cursor was at the start of the post, and there was a space between the cursor and the tag, because I think (and this is just a guess) most people would want to put the tags at the end.

So if | represents the cursor, currently it's:

#hashtag|

but it should be:

| #hashtag

Then the user can immediately start typing, and the tag will appear at the end of their post.

What do you think?

If you agree, I'd do this by (untested):

  1. Creating an enum to represent the initial cursor position;
enum class InitialCursorPosition {
    START,
    END,
}
  1. Add the cursor position as a field to ComposeOptions, with a default of END to keep the current behaviour.

  2. Set the cursor position in the compose options for a hashtag to START.

  3. Add a space immediately before the hashtag in the compose options content.

  4. Set the cursor position in ComposeActivity.onCreate(), probably immediately after this code, binding.composeEditField.setText(statusContent)

It's a bit more work, but it might be a nicer user experience. How do you feel about that?

@sanao1006
Copy link
Contributor Author

sanao1006 commented Nov 17, 2023

I suspect it would be better if the cursor was at the start of the post, and there was a space between the cursor and the tag, because I think (and this is just a guess) most people would want to put the tags at the end.

I agree!

Add the cursor position as a field to ComposeOptions, with a default of END to keep the current behaviour.

Does this mean that by default the cursor position is like #hashtag |?

That is just my feeling, but by default, the cursor position is more comfortable with START (e.g. | #hashtag.).
However, this may be a difference in cultural sensibilities, so I respect Nik's opinion (Because Mastodon has many English-speaking users)!

@sanao1006
Copy link
Contributor Author

Oh, sorry, I may have misunderstood.

I mistakenly thought you were going to allow the users to set the cursor's initial position at START(e.g. | #hashtag) and END (e.g. #hashtag |)
And I was wondering if you were suggesting that the initial position be END (e.g. #hashtag |).

You simply suggested setting the initialCursorPosition to be added to ComposeOptions with an initial value of END as follows

        ...
        var statusId: String? = null,.
        var kind: ComposeKind? = null,.
        var initialCursorPosition: InitialCursorPosition = InitialCursorPosition.END
    ) : Parcelable

And you intended to set the initial value of StatusListActivity to START as follows, right?

ComposeActivity.startIntent(
   this,
   ComposeActivity.ComposeOptions(
       content = getString(R.string.title_tag).format(tag),
       initialCursorPosition = ComposeActivity.InitialCursorPosition.START,
),

(And then insert a space after this)

But, This interpretation may also be incorrect, so I would like to reconcile the perceptions between you and me!

@nikclayton
Copy link
Contributor

Oh, sorry, I may have misunderstood.

I mistakenly thought you were going to allow the users to set the cursor's initial position at START(e.g. | #hashtag) and END (e.g. #hashtag |) And I was wondering if you were suggesting that the initial position be END (e.g. #hashtag |).

You simply suggested setting the initialCursorPosition to be added to ComposeOptions with an initial value of END as follows

        ...
        var statusId: String? = null,.
        var kind: ComposeKind? = null,.
        var initialCursorPosition: InitialCursorPosition = InitialCursorPosition.END
    ) : Parcelable

And you intended to set the initial value of StatusListActivity to START as follows, right?

ComposeActivity.startIntent(
   this,
   ComposeActivity.ComposeOptions(
       content = getString(R.string.title_tag).format(tag),
       initialCursorPosition = ComposeActivity.InitialCursorPosition.START,
),

(And then insert a space after this)

But, This interpretation may also be incorrect, so I would like to reconcile the perceptions between you and me!

Yes, that's right.

So the default behaviour when creating a post is that the cursor is at the end, except when creating a post from a hashtag, where the cursor is at the start.

In that last example I think you can just embed the space directly in content, so it would be:

ComposeActivity.startIntent(
   this,
   ComposeActivity.ComposeOptions(
       content = " ${getString(R.string.title_tag).format(tag)}",
       initialCursorPosition = ComposeActivity.InitialCursorPosition.START,
),

(or, instead of reusing R.string.title_tag, create a new string resource just for this use that contains the embedded space in the right place).

sanao1006 and others added 2 commits November 17, 2023 20:53
…en the compose button is clicked in a hashtag timeline

The previous code positioned the cursor in EditText at the right edge when the compose button was pressed from the hashtag timeline. So This was inconvenient because the cursor had to be moved to the left edge in order to enter text.

Therefore, solve this by positioning the initial position of the cursor at the left end.

Fixes pachli#254, pachli#228

----------------------
Co-authored-by: Nik Clayton <nik@ngo.org.uk>
@sanao1006
Copy link
Contributor Author

@nikclayton

Hi, please review!

@@ -285,6 +285,8 @@ class ComposeActivity :
binding.composeEditField.post {
binding.composeEditField.requestFocus()
}

binding.composeEditField.setSelection(0)
Copy link
Contributor

Choose a reason for hiding this comment

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

This isn't quite right, because it always places the cursor at the start. You can see this if you try and reply to a post -- previously you'd get a reply that starts with the addresses of the people being replied to, and then the cursor at the end.

Now you get the list of people being replied to and the cursor is at the start.

If you check the existing code you'll see there's a function called setupComposeField() which does this. At the moment it has a line:

binding.composeEditField.setSelection(binding.composeEditField.length())

which puts the cursor at the end, but your code here overrides that.

So what I think you need to do is:

  1. Remove this line here
  2. Add a composeOptions: ComposeOptions? parameter to setupComposeField().
  3. Pass this parameter when setupComposeField() is called (around line 260)

Then you can replace the

binding.composeEditField.setSelection(binding.composeEditField.length())

in setupComposeField() with something like:

when (composeOptions?.initialCursorPosition ?: InitialCursorPosition.END) {
    InitialCursorPosition.START -> binding.composeEditField.setSelection(0)
    InitialCursorPosition.END -> binding.composeEditField.setSelection(
        binding.composeEditField.length()
    )
}

That way the initialCursorPosition value is honoured when the compose field is set up, and the default value is END which matches the current behaviour.

Does that make sense?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Does that make sense?

Yes, I understand.

Sorry, I did not take replies into consideration.
It is indeed inconvenient because the current code makes the cursor go to the START position in reply.

Thank you for pointing this out!

sanao1006 and others added 2 commits November 19, 2023 17:16
…ing a reply.

The previous code positioned the EditText cursor to the left when creating a reply.
However, the cursor in Edittext should only be positioned on the left edge when creating a toot in the hashtag timeline.

Therefore, set the cursor of EditText to be positioned at the right edge when creating a reply.

Fixes pachli#254, pachli#228

----------------------
Co-authored-by: Nik Clayton <nik@ngo.org.uk>
…n_trending_tag_timeline' into 228-display_the_compose_button_on_trending_tag_timeline
@sanao1006
Copy link
Contributor Author

Behavior

reply hashtag
Screen_recording_20231119_174142.webm
Screen_recording_20231119_174022.webm

@sanao1006 sanao1006 requested a review from nikclayton November 19, 2023 08:48
@nikclayton nikclayton merged commit adb9fcf into pachli:main Nov 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants