Display Notes on Activity view #2292
Conversation
shubhamkmr04
commented
Jul 18, 2024
•
shubhamkmr04
commented
shubhamkmr04
force-pushed
the
shubham/upgrade-notes-feature
branch
from
a7504db
to
daa11f9
Compare
shubhamkmr04
force-pushed
the
shubham/upgrade-notes-feature
branch
from
daa11f9
to
b1316e6
Compare
Fixed the notes overlapping and the char limit to 150, you think it would work ? |
|
Looking good now, visually.
|
| @@ -345,6 +349,8 @@ export default class Wallet extends React.Component<WalletProps, WalletState> { | |||
|
|
|||
| LnurlPayStore.reset(); | |||
|
|
|||
| NotesStore?.loadNoteKeys(); | |||
There was a problem hiding this comment.
I don't believe we need to load this every time we hit the wallet screen. On connecting should suffice
There was a problem hiding this comment.
Yup, we have covered that in 2393
|
We need to rebase this too, once ready. Please rewind your commits and rebase instead of doing a merge commit, please. |
shubhamkmr04
force-pushed
the
shubham/upgrade-notes-feature
branch
4 times, most recently
from
8ff79ac
to
4739416
Compare
views/AddNotes.tsx
Outdated
| const key: any = | ||
| this.state.txid || | ||
| this.state.payment_hash || | ||
| this.state.getRPreimage; |
There was a problem hiding this comment.
why this change? you're loosening the typing and seemingly breaking the fetching of the key here
There was a problem hiding this comment.
we were getting type error here like Type 'string | undefined' is not assignable to type 'string'. Type 'undefined' is not assignable to type 'string'. once I removed here- because it was already coming in getNoteKey
views/AddNotes.tsx
Outdated
| @@ -71,8 +70,7 @@ export default class AddNotes extends React.Component< | |||
| const { notes } = this.state; | |||
|
|
|||
| const saveNote = async () => { | |||
| const key: string = | |||
| 'note-' + (payment_hash || txid || getRPreimage); | |||
| const key: any = payment_hash || txid || getRPreimage; | |||
views/AddNotes.tsx
Outdated
| 'note-' + | ||
| (payment_hash || txid || getRPreimage); | ||
| const key: any = | ||
| payment_hash || txid || getRPreimage; |
There was a problem hiding this comment.
same here. I do not understand the changes to this file
views/Invoice.tsx
Outdated
| @@ -91,7 +91,9 @@ export default class InvoiceView extends React.Component<InvoiceProps> { | |||
| const EditNotesButton = () => ( | |||
| <TouchableOpacity | |||
| onPress={() => | |||
| navigation.navigate('AddNotes', { getRPreimage: noteKey }) | |||
| navigation.navigate('AddNotes', { | |||
| getRPreimage: getNoteKey | |||
There was a problem hiding this comment.
I really dislike the name of the prop we're passing in here. Doesn't it work better as noteKey or something akin to that?
views/Payment.tsx
Outdated
| getPaymentRequest | ||
| } = payment; | ||
| const date = getDisplayTime; | ||
|
|
||
| const EditNotesButton = () => ( | ||
| <TouchableOpacity | ||
| onPress={() => | ||
| navigation.navigate('AddNotes', { payment_hash: noteKey }) | ||
| navigation.navigate('AddNotes', { | ||
| payment_hash: getNoteKey |
There was a problem hiding this comment.
same here with the prop name here. do we handle these differently?
nit: also confused by the different casing
… in all views while sending it to AddNotes
shubhamkmr04
force-pushed
the
shubham/upgrade-notes-feature
branch
from
481d980
to
286f615
Compare
