winglang / wing

A programming language for the cloud ☁️ A unified programming model, combining infrastructure and runtime code into one language ⚡
https://winglang.io
Other
4.76k stars 187 forks source link

feat(console): wing console application reset capability #6665

Closed polamoros closed 2 weeks ago

polamoros commented 3 weeks ago

Resolves https://github.com/winglang/wing/issues/6124

github-actions[bot] commented 3 weeks ago

Thanks for opening this pull request! :tada: Please consult the contributing guidelines for details on how to contribute to this project. If you need any assistance, don't hesitate to ping the relevant owner over Discord.

Topic Owner
Wing SDK and utility APIs @chriscbr
Wing Console @ainvoner, @skyrpex, @polamoros
JSON, structs, primitives and collections @hasanaburayyan
Platforms and plugins @hasanaburayyan
Frontend resources (website, react, etc) @tsuf239
Language design @chriscbr
VSCode extension and language server @markmcculloh
Compiler architecture, inflights, lifting @yoav-steinberg
Wing Testing Framework @tsuf239
Wing CLI @markmcculloh
Build system, dev environment, releases @markmcculloh
Library Ecosystem @chriscbr
Documentation @hasanaburayyan
SDK test suite @tsuf239
Examples @hasanaburayyan
Wing Playground @eladcon
monadabot commented 3 weeks ago

Console preview environment is available at https://wing-console-pr-6665.fly.dev :rocket:

Last Updated (UTC) 2024-06-14 08:50
polamoros commented 3 weeks ago

I am adding a header with a 'Restart simulator' button on the left and moving the Discord button from the footer to the right side of the header.

Thoughts?

@skyrpex @ainvoner @eladb

image image
polamoros commented 3 weeks ago

@polamoros I am not a fan of the new header.

I think the "RESET" button should be on the top right of the map view:

Screenshot 2024-06-10 at 16 20 31

And it should just say "Reset".

When clicking on it, there should be a message box:

Are you sure you want to reset all state and restart the application?

YES/NO

The Discord button can stay on the footer AFAIK for now.

I don't like the header either. Regarding moving the reset button to the map controls, I don't think it's the correct solution. We are restarting the application, not just the map. I would prefer moving it to the footer or adding a menu on the right side of the header (if we decide to keep the header).

Here's an example image

Regarding the Discord button, @ainvoner wants to place it in a more visible location.

skyrpex commented 3 weeks ago

@polamoros I am not a fan of the new header.

I think the "RESET" button should be on the top right of the map view:

Screenshot 2024-06-10 at 16 20 31

And it should just say "Reset".

When clicking on it, there should be a message box:

Are you sure you want to reset all state and restart the application?

YES/NO

The Discord button can stay on the footer AFAIK for now.

It doesn't make sense to me to have such a functionality inside the map view: the reset action affects the whole application, not just the map view. Also, it may signal to the users that it only resets the view (same as the [] button on the right side).

Even if a new header is almost empty, it makes sense to place the reset button there IMO.

polamoros commented 2 weeks ago

What about moving it to the left side of the footer?

I think it makes more sense.

image image

@eladb @skyrpex @ainvoner

skyrpex commented 2 weeks ago

We could also merge the functionality of the reset button and the Status label. They are similar.

skyrpex commented 2 weeks ago

It's strange that the Status label isn't updated when the app is restarting 🤔

skyrpex commented 2 weeks ago

I think the restart button should be also disabled when the simulator is loading (e.g. after editing the wing file).

mergify[bot] commented 2 weeks ago

Thanks for contributing, @polamoros! This PR will now be added to the merge queue, or immediately merged if 6124-wing-console-application-reset-capability is up-to-date with main and the queue is empty.

mergify[bot] commented 2 weeks ago

Thanks for contributing, @polamoros! This PR will now be added to the merge queue, or immediately merged if 6124-wing-console-application-reset-capability is up-to-date with main and the queue is empty.

monadabot commented 2 weeks ago

Congrats! :rocket: This was released in Wing 0.74.60.