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

add note field && implement saveButton to editPlanet #12

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

misarb
Copy link

@misarb misarb commented Aug 19, 2022

add field note , and implement a save Button in Edit page

@abertschi
Copy link
Owner

Hello there, thank you for your contributions 👍

There are a couple of things which have to be improved before I want to merge this into master.

  • There is no need for a 'save' button, all modifications are already synced with the model on a change.
  • There are typos across the PR with plant: you write planet instead of plant.
  • one of the main goals of this app is to keep the UI as minimalistic as possible, while showing a realistic usage of the provider package. many tutorials out there show a too simplistic 'hello world' scenario to demonstrate the provider package. maybe you find a nicer way to integrate the note into the rest of the UI. for instance, if no note is added, you show a red banner with 'add a note in edite' which I think is rather intrusive.

I can look into your changes and try to come up with a better UI integration if I find the time, or you try out more ideas. Nonetheless, many thanks for your PR.

image

@misarb
Copy link
Author

misarb commented Aug 22, 2022

hello @abertschi thank you for this explanation that I appreciate,i set now the case when there is no note, and I change the planet with the plant it was an error when I write :), and i will try if i can find a better UI integration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants