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

35: watchlist shows OTH values #380

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

orfins
Copy link

@orfins orfins commented Jul 18, 2024

This PR adds OTH values to the watchlist tab only (will not be seen in the widget)

*This is my first time developing an app, so please let me know of any changes I should make. For example, I do need some help with db migrations - they don't seem to run on startup.

This is what the Watchlist tab looks like with the changes:

Untitled

@@ -2,11 +2,11 @@
"formatVersion": 1,
"database": {
"version": 6,
"identityHash": "a9411595d733a02f3a0d29b7aba56959",
Copy link
Owner

Choose a reason for hiding this comment

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

Prefer this to be untouched and a new version 7 schema be added to this folder

Copy link
Author

Choose a reason for hiding this comment

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

Updated. I still need some help with making sure the 6->7 migration runs :)

app:layout_constraintTop_toBottomOf="@+id/name" />

<EditText
android:id="@+id/othText"
Copy link
Owner

Choose a reason for hiding this comment

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

The positions layout (item_position.xml) is unchanged. Does it also need these changes or is it ok to not include marked closed text?

Copy link
Author

Choose a reason for hiding this comment

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

Personally I'd rather see both "At close" and "Market closed" values if that's what you're asking. If you want to remove the titles themselves then sure, but we need to make sure it's clear which values represent what (open/close/at close).

android:background="@null"
android:ems="10"
android:inputType="text"
android:text="Market Closed:"
Copy link
Owner

Choose a reason for hiding this comment

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

the text needs to be translated across languages - can you use strings.xml for all the new text fields?

Copy link
Author

Choose a reason for hiding this comment

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

Updated. Used Google Translate so I don't know if the translations are good enough.

@orfins orfins force-pushed the feat/oth-values-in-watchlist branch 2 times, most recently from 5378ae5 to 01f5c26 Compare July 23, 2024 16:56
@orfins orfins force-pushed the feat/oth-values-in-watchlist branch from 01f5c26 to b16123d Compare July 23, 2024 16:57
Repository owner locked and limited conversation to collaborators Nov 15, 2024
Repository owner deleted a comment from DiagonalArg Nov 15, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants