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

chore: initial version #1

Merged
merged 1 commit into from Feb 4, 2020
Merged

chore: initial version #1

merged 1 commit into from Feb 4, 2020

Conversation

@Lemii
Copy link
Contributor

Lemii commented Feb 4, 2020

This PR pushes the the initial version (0.8.0) of the ARK messenger client to the repository.

@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 4, 2020

Thanks for submitting this pull request! A maintainer will review this in the next few days and explicitly select labels so you know what's going on.

If no reviewer appears after a week, a reminder will be sent out.

@faustbrian faustbrian changed the title release: push initial version chore: initial version Feb 4, 2020
@faustbrian faustbrian merged commit 31c0c06 into ArkEcosystem:master Feb 4, 2020
@ArkEcosystemBot

This comment has been minimized.

Copy link
Member

ArkEcosystemBot commented Feb 4, 2020

Your pull request has been merged but was not assigned a bounty tier. A maintainer will assign a bounty tier to this pull request in the next few days.

@@ -0,0 +1,56 @@
describe('Channels', () => {

This comment has been minimized.

Copy link
@faustbrian

faustbrian Feb 7, 2020

Contributor

The whole __tests__ folder should be moved up to the top-level.

This comment has been minimized.

Copy link
@Lemii

Lemii Feb 10, 2020

Author Contributor

@faustbrian The project is bootstrapped with Create React App and requires the __tests__ folder to be placed inside the src folder. If you move the folder to the root it will create all kinds of problems and would need quite some extra work to get it working properly. Worst case scenario an eject is necessary, and this would not work well with the goals set out for this project.

src/utils/api.ts Show resolved Hide resolved
src/utils/formatters.ts Show resolved Hide resolved
src/utils/formatters.ts Show resolved Hide resolved
src/utils/formatters.ts Show resolved Hide resolved
src/utils/storage.ts Show resolved Hide resolved
@faustbrian

This comment has been minimized.

Copy link
Contributor

faustbrian commented Feb 7, 2020

  • If safari is still an issue because of BigInt, it makes sense to add that disclaimer somewhere in the readme.
  • Sending more than 1 message in a short timeframe (e.g. a b c d e) discards most of the messages after a. Is it only possible to have 1 transaction in a block or what is the cause of this?
  • Linebreaks are not send over, so you can add those to your message but after sending it will all be on one line.
  • Not sure what the message preview modal is for. I expected a preview of a message after it was sent and not forged yet instead of a modal that I can click before sending.
  • Sending <test> returns in an empty message as it's not rendered (added to the DOM instead). This also means you can send stuff like <span style="color: rgb(252, 19, 3);">oh no</span> and it will show up as html (needs some sanitizing)
  • You can send a message to any existing chat address and it will show up in there.
@Lemii

This comment has been minimized.

Copy link
Contributor Author

Lemii commented Feb 7, 2020

The app supports Markdown. So if you apply markdown syntax to your message input, you can preview it with this modal. Perhaps I should rename the preview to "Markdown preview" (it will also serve as a small hint that Markdown is supported that way)

  • If safari is still an issue because of BigInt, it makes sense to add that disclaimer somewhere in the readme.

Will do.

  • Sending more than 1 message in a short timeframe (e.g. a b c d e) discards most of the messages after a. Is it only possible to have 1 transaction in a block or what is the cause of this?

I believe it's a small error in the custom transaction, where it rejects a tx if the sender already is in the pool (forgot to omit it when using the register business transaction as a template)

  • Linebreaks are not send over, so you can add those to your message but after sending it will all be on one line.

Will look into it.

  • Not sure what the message preview modal is for. I expected a preview of a message after it was sent and not forged yet instead of a modal that I can click before sending.

The app supports Markdown. So if you apply markdown syntax to your message input, you can preview it with this modal. Perhaps I should rename the preview to "Markdown preview" (it will also serve as a small hint that Markdown is supported that way)

  • Sending <test> returns in an empty message as it's not rendered (added to the DOM instead). This also means you can send stuff like <span style="color: rgb(252, 19, 3);">oh no</span> and it will show up as html (needs some sanitizing)

Will do.

  • You can send a message to any existing chat address and it will show up in there.

This is true. However, as discussed in the chat with Michel a little while ago, it is indeed an issue, but also considered out of scope for now.

Thanks for the extensive feedback and will get back to it soon! 👍

@Lemii Lemii mentioned this pull request Feb 10, 2020
faustbrian pushed a commit that referenced this pull request Feb 10, 2020
chore: initial version (#1)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants
You can’t perform that action at this time.