[2] Add TUI messages#103

JonoPrest · 2024-08-02T09:42:31Z

See screenshots below demoing multiple messages of different types, failure to load message and current message that is deployed with the API

JonoPrest · 2024-08-02T09:44:11Z

+    @as("version") envioVersion: string,
+    @as("apiToken") hasApiToken: bool,
+    usesHyperSync: bool,
+    config: ConfigYAML.t,


@DZakh note I aded "usesHyperSync" field so no extra parsing is needed on the server side yet. I'm still sending across the reduced ConfigYAML (redacted addresses and rpc secrets etc) to allow us to parse info if we need for messaging later on.

JonoPrest · 2024-08-02T09:51:06Z

+    | Err(_) =>
+      <Message message={kind: Destructive, content: "Failed to load messages from envio server"} />
+    }}


Do we actually want to show that there's a failure? It's not important to display if there is but my only concern is it would go by undetected as it would present the same as no messages at all.

Maybe we should implement something like envio.dev/app/hyperindex/send-error and integrate it with Sentry?

JonoPrest · 2024-08-02T10:02:34Z

-Dotenv.initialize()
-


Dotenv now gets initialized in the Env.res file before safe Env.

JonoPrest · 2024-08-02T10:04:35Z

+  S.object(s => {
+    s.tag("kind", "Rpc")
+    //Do not share users private rpc details
+    Rpc(Js.Nullable.Null->Utils.magic)
+  }),


@DZakh maybe you have some wisdom for how this should look? I want to be able to serialize this to share that it is using an RPC without any of the other values associated with the provider (in this case just a URL)

It's obviously not deserializeable with the same schema but currently don't need deserialization

DZakh · 2024-08-02T13:12:11Z

+    envioVersion: s.field("version", S.string),
+    hasApiToken: s.fieldOr("apiToken", S.bool, false),
+    usesHyperSync: s.field("usesHyperSync", S.bool),
+    config: s.field("config", ConfigYAML.schema),


Suggested change

envioVersion: s.field("version", S.string),

hasApiToken: s.fieldOr("apiToken", S.bool, false),

usesHyperSync: s.field("usesHyperSync", S.bool),

config: s.field("config", ConfigYAML.schema),

envioVersion: s.field("version", S.string),

apiToken: s.field("apiToken", S.option<S.string>), // Send the token itself

hyperSync: s.field("hyperSync", S.bool), // I don't like the uses prefix for some reason :) How about simply sending hyperSync or poweredByHyperSync?

// config: s.field("config", ConfigYAML.schema),

// Instead of the schema we decided to send:

ecosystem: s.field("ecosystem", S.union([S.literal("evm"), S.literal("fuel")])),

chainIds: s.field("chainIds", S.array(S.int))

DZakh · 2024-08-02T13:18:17Z

+      PersistedState.getPersistedState()->Belt.Result.mapWithDefault("unknown", ps =>
+        ps.envioVersion
+      )


Maybe we should better send it as optional?

Sure, it just needs to be updated on the server as well

DZakh · 2024-08-02T13:19:09Z

 let makeAppState = (globalState: GlobalState.t): EnvioInkApp.appState => {
  open Belt
  {
+    config: globalState.config->ConfigYAML.fromConfig(~shouldRemoveAddresses=true),


Let's not send config and remove all the parsing

DZakh · 2024-08-02T13:25:57Z

I think it would be nice to highlight them a little bit more somehow. Either by adding a space below hasura link, or adding a big title saying Notifications/Messages, or maybe adding an icon. I'm not very familiar with Ink

* Add hypersync api key environment variable * Make env api token optional and default in client

Rename hypersyncApiToken to envioApiToken

JonoPrest · 2024-08-02T16:28:22Z

Updated look for notifications:

DZakh

Looks good. The only think I'd like to have is a white colored message.

And, also I think it's good to design messages compatible to Ink API:

type message = {
  kind: "text"
  color: "primary" | "secondary" | "info" | "danger" | ...
  content: string
}

So it's easier to understand which color the notification will has, and gives us a possibility to add more complex notifications in the future.

And I left a comment on the ecosystem format we want to pass, which will also require change in the PR.

DZakh · 2024-08-02T16:43:46Z

+    }
+  }
+
+  type messageKind = | @as("warning") Warning | @as("destructive") Destructive | @as("info") Info


Let's also add a general message to use it as fallback, in case we want to add a new kind in the future. For example, we can have it white.

DZakh · 2024-08-02T16:44:32Z

+  }
+
+  let messageSchema = S.object(s => {
+    kind: s.field("kind", S.union([S.literal(Warning), S.literal(Destructive), S.literal(Info)])),


Suggested change

kind: s.field("kind", S.union([S.literal(Warning), S.literal(Destructive), S.literal(Info)])),

kind: s.field("kind", S.union([S.literal(Warning), S.literal(Destructive), S.literal(Info), S.string->S.variant(_ => General)])),

Ah, well, we can handle it on the server, since we know the version. But I think it's still nice to have a white colored message.

DZakh · 2024-08-02T16:47:06Z

+      | Warning => Secondary
+      | Info => Info
+      | Destructive => Danger


Let's use the same names as Ink does, so it makes more sense? 🤔

Yeah lets do that 👍🏼

DZakh

Nice 👍

DZakh · 2024-08-05T11:15:41Z

+  let toTheme = (color: messageColor): Style.chalkTheme =>
+    switch color {


Does :> work here?

They are not the same underlying string though...

Ah, different casing or something

JonoPrest commented Aug 2, 2024

View reviewed changes

JonoPrest changed the title ~~Add TUI messages~~ [2] Add TUI messages Aug 2, 2024

JonoPrest requested a review from DZakh August 2, 2024 09:49

JonoPrest commented Aug 2, 2024

View reviewed changes

JonoPrest force-pushed the jp/init-api-key branch from 2c4e448 to 9012eab Compare August 2, 2024 10:00

JonoPrest force-pushed the jp/add-tui-messages branch from c0c4c8e to 293939a Compare August 2, 2024 10:01

JonoPrest commented Aug 2, 2024

View reviewed changes

JonoPrest force-pushed the jp/init-api-key branch from bd8e843 to e2a4082 Compare August 2, 2024 12:26

JonoPrest force-pushed the jp/add-tui-messages branch 2 times, most recently from 0a0284f to eb25e09 Compare August 2, 2024 12:44

Base automatically changed from jp/init-api-key to main August 2, 2024 13:00

DZakh reviewed Aug 2, 2024

View reviewed changes

JonoPrest and others added 7 commits August 2, 2024 18:16

Add support for dotenv

51ffd74

Rename handlers package to root in generated

8347093

Update lock files

282d1e0

[2] Add hypersync api key environment variable (#71)

7f6045d

* Add hypersync api key environment variable * Make env api token optional and default in client

Remove unused rescript/core package

3d8e060

Implement tui component for displaying messages

0b8e21f

Rename hypersyncApiToken to envioApiToken

Remove unused sanitize function

e8a8803

JonoPrest force-pushed the jp/add-tui-messages branch from eb25e09 to 9a1b3eb Compare August 2, 2024 16:17

Refactor body content and notifications heading

8649b8d

DZakh approved these changes Aug 2, 2024

View reviewed changes

Use same naming convention for message kind as theme

1c71612

JonoPrest force-pushed the jp/add-tui-messages branch from 9a1b3eb to 1c71612 Compare August 5, 2024 09:35

Lowercase ecosystem tag

c9767b7

Rename kind to color

7e59729

JonoPrest enabled auto-merge (squash) August 5, 2024 11:04

DZakh approved these changes Aug 5, 2024

View reviewed changes

JonoPrest merged commit 97a3a19 into main Aug 5, 2024

JonoPrest deleted the jp/add-tui-messages branch August 5, 2024 14:59

-    envioVersion: s.field("version", S.string),
-    hasApiToken: s.fieldOr("apiToken", S.bool, false),
-    usesHyperSync: s.field("usesHyperSync", S.bool),
-    config: s.field("config", ConfigYAML.schema),
+    envioVersion: s.field("version", S.string),
+    apiToken: s.field("apiToken", S.option<S.string>), // Send the token itself
+    hyperSync: s.field("hyperSync", S.bool), // I don't like the uses prefix for some reason :) How about simply sending hyperSync or poweredByHyperSync?
+    // config: s.field("config", ConfigYAML.schema),
+    // Instead of the schema we decided to send:
+    ecosystem: s.field("ecosystem", S.union([S.literal("evm"), S.literal("fuel")])),
+    chainIds: s.field("chainIds", S.array(S.int))

	kind: s.field("kind", S.union([S.literal(Warning), S.literal(Destructive), S.literal(Info)])),
	kind: s.field("kind", S.union([S.literal(Warning), S.literal(Destructive), S.literal(Info), S.string->S.variant(_ => General)])),

		let toTheme = (color: messageColor): Style.chalkTheme =>
		switch color {

		Dotenv.initialize()

Uh oh!

Conversation

JonoPrest commented Aug 2, 2024

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DZakh commented Aug 2, 2024

Uh oh!

JonoPrest commented Aug 2, 2024

Uh oh!

DZakh left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DZakh left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

DZakh left a comment •

edited

Loading