Skip to content
This repository has been archived by the owner on Apr 25, 2023. It is now read-only.

Set name and label at creation of client. Fixes #48 #49

Merged

Conversation

spetzreborn
Copy link
Contributor

Changes behaviour of the add button from directly creating an
configuration to directing the user to an "create configuration" view.

If no name is given the default name is used same way as before.
This means that the go backend works with the old frontend as well with the new.

Changes behaviour of the add button from directly creating an
configuration to directing the user to an "create configuration" view.

If no name is given the default name is used same way as before.
This means that the go backend works with the old frontend as well with the new.
1. Fix bug introduced in last commit.
Don't try to json encode data that is already encoded.

2. Honor --max-number-client-config
When the creation moved from Clients.svelte to NewClient.svelte the
check in returned json for errors got lost. Now it is back.
@spetzreborn
Copy link
Contributor Author

I've introduced 2 bugs in the PR, but fixed them in 7d1c38b I did not want to amend or squash them as the PR was alive. But if you want to merge I suggest they be squashed together.

I have tried to fix the error checks, but I do not know that the docker build is failing on.
The file permission or bindata_assetfs I have not touched in this pull request.

Copy link
Contributor

@luna-duclos luna-duclos left a comment

Choose a reason for hiding this comment

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

The code all looks reasonable

@luna-duclos luna-duclos merged commit ffeb822 into EmbarkStudios:master Apr 14, 2020
@luna-duclos
Copy link
Contributor

Thank you for your contribution!

@spetzreborn spetzreborn deleted the name_client_config_at_creation branch April 15, 2020 05:22
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.

3 participants