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

feat: support naming on pod and scope #129

Merged
merged 1 commit into from
Dec 9, 2022
Merged

Conversation

pionxzh
Copy link
Contributor

@pionxzh pionxzh commented Dec 9, 2022

closes #97

long text will become an ellipsis.
image

We can further improve some of the controlling behaviors around this name input field. (hint, style, arrow key navigation)

@forrestbao
Copy link
Collaborator

@pionxzh what do you mean by hint and style?
I love the arrow key idea.

@li-xin-yi
Copy link
Collaborator

How do we intend to manage the name state?

It seems that you only maintain the pod names during one-time visit life-cycle in this PR.

I just wonder which mode we should design

  • keep the consistent pod names for all users all the time ( sync by DB and yjs server)
  • or, let each user tags the pod with their preferred name even in one same repo
  • or, reset the pod names by default after refreshing

It involves some back-end DB and yjs server works. I can work on it after we agree on either mode.

@pionxzh
Copy link
Contributor Author

pionxzh commented Dec 9, 2022

How do we intend to manage the name state?

It seems that you only maintain the pod names during one-time visit life-cycle in this PR.

I just wonder which mode we should design

  • keep the consistent pod names for all users all the time ( sync by DB and yjs server)
  • or, let each user tags the pod with their preferred name even in one same repo
  • or, reset the pod names by default after refreshing

It involves some back-end DB and yjs server works. I can work on it after we agree on either mode.

Good point. Actually I didn't check did it really get persisten to the DB or not, but it did remember my changes after reloading.

I prefer the idea of syncing pod name to DB and YJS.

@pionxzh
Copy link
Contributor Author

pionxzh commented Dec 9, 2022

@pionxzh what do you mean by hint and style?
I love the arrow key idea.

Oh I mean the user interaction things like

  1. hover/focus effect
  2. Do we need additional styles on the name?

And some problems like:

  1. Disable node dragging on the name? (Which allows selection editing on the name)

@lihebi
Copy link
Collaborator

lihebi commented Dec 9, 2022

it did remember my changes after reloading.

Yes, it is persisted to DB, as you re-used the (currently unused but synced) name field of the pod (which is clever!).

This PR is good for now, I'm gonna merge it. we can later add yjs sync amount shared users in another PR (using "sync by DB and yjs server" design). Not too urgent though, @li-xin-yi if you have time, feel free to take lead on this.

Copy link
Collaborator

@lihebi lihebi left a comment

Choose a reason for hiding this comment

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

Great feature, thanks Pionxzh!

@lihebi lihebi merged commit c0a63cd into codepod-io:main Dec 9, 2022
li-xin-yi pushed a commit to li-xin-yi/codepod-1 that referenced this pull request Dec 10, 2022
li-xin-yi pushed a commit to li-xin-yi/codepod-1 that referenced this pull request Dec 10, 2022
li-xin-yi pushed a commit to li-xin-yi/codepod-1 that referenced this pull request Dec 10, 2022
@forrestbao
Copy link
Collaborator

@pionxzh the arrow key idea was mentioned here: #87 I will temporarily assign to you.

@pionxzh pionxzh deleted the node-name branch December 10, 2022 15:16
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.

Allowing naming pods and scopes
4 participants