Skip to content

Conversation

@chrjsorg
Copy link
Contributor

@chrjsorg chrjsorg commented Nov 20, 2017

  • Setting included (default: deactivated)
  • Translation for en/de

refs #40

What do you think? Did I miss anything?

Cheers,
Chris

- Setting included (default: deactived)
- Translation for en/de

refs SimpleMobileTools#40
@tibbi
Copy link
Contributor

tibbi commented Nov 20, 2017

nice, I will test the functionality a bit later. For now just add the new string in every strings file (even if obviously not translated), add an ID field to the new RelativeLayout at fragment_note and remove all comments, they arent necessary. Thanks

Copy link
Contributor

@trubitsyn trubitsyn left a comment

Choose a reason for hiding this comment

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

Nice work, left some comments.

android:layout_width="wrap_content"
android:layout_height="wrap_content"
android:textStyle="italic"
android:text="123"
Copy link
Contributor

Choose a reason for hiding this comment

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

You could safely remove this unused attribute

<string name="place_cursor_end">Place cursor to the end of note</string>
<string name="monospaced_font">Use monospaced font</string>
<string name="show_keyboard">Show keyboard on startup</string>
<string name="show_wordcount">Show word counter</string>
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe show_word_count would be better.

val WIDGET_NOTE_ID = "widget_note_id"
val MONOSPACED_FONT = "monospaced_font"
val SHOW_KEYBOARD = "show_keyboard"
val SHOW_WORDCOUNT = "show_word_count"
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe SHOW_WORD_COUNT would be better

}

private fun setWordCounter(text: Editable) {
//Replace new lines with space
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess everyone can guess what's going on, no need to add a comment here.


<RelativeLayout
android:layout_width="match_parent"
android:id="@+id/notes_relativelayout"
Copy link
Contributor

Choose a reason for hiding this comment

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

notes_relative_layout would keep the code style unified

android:layout_height="wrap_content"
android:textStyle="italic"
android:text="123"
android:text=""
Copy link
Contributor

Choose a reason for hiding this comment

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

lets make this: tools:text="123"
that way it will be shown in android studio, but wont be rendered in the app

Copy link
Contributor

Choose a reason for hiding this comment

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

What's the purpose of setting this attribute? It is overwritten anyway. Am I missing something?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I added tools:text, in case you missed it because I edited the file meanwhile.

Copy link
Contributor

Choose a reason for hiding this comment

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

the purpose is just to see that view in the editor, in android studio

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah, I get it now.

- changes string name to show_word_count
- changed id of relative layout to notes_relative_layout

refs SimpleMobileTools#141
@trubitsyn
Copy link
Contributor

@tibbi I think Show word count is better than Show word counter, because while it really is a word counter internally, everybody expects to see word count, not what it does, but what it is. I can't speak for other languages, but in Russian it feels very unnatural to call it counter.

@tibbi
Copy link
Contributor

tibbi commented Nov 20, 2017

ye make it "word count" please, like the strings key name

@tibbi tibbi merged commit 7ecde39 into SimpleMobileTools:master Nov 21, 2017
@tibbi
Copy link
Contributor

tibbi commented Nov 21, 2017

ye seems to be working nice, thanks. Ive just added a couple smaller improvements

@chrjsorg chrjsorg deleted the feature/chrjsorg/40-word-count branch November 21, 2017 12:05
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