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.

Sign up for GitHub

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

Jump to bottom

Indicate to the user when text is copied to the clipboard #642

Merged
merged 7 commits into from
Aug 23, 2021
Merged

Indicate to the user when text is copied to the clipboard #642

merged 7 commits into from
Aug 23, 2021

Conversation

tlinford
Copy link
Contributor

Resolves #640

  • broadcast CopyToClipboard event to plugins after selection has been
    copied, and InputReceived event after any input has been received.
  • add new ClientToServerMsg InputReceived
  • subscribe status-bar plugin to new events, modify second line after
    CopyToClipboard, reset it on InputReceived.

@tlinford
Indicate to the user when text is copied to the clipboard
86e0b98 
- broadcast CopyToClipboard event to plugins after selection has been
  copied, and InputReceived event after any input has been received.
- add new ClientToServerMsg InputReceived
- subscribe status-bar plugin to new events, modify second line after
  CopyToClipboard, reset it on InputReceived.
imsnif
imsnif requested changes Aug 16, 2021
View reviewed changes
Copy link
Member

@imsnif imsnif left a comment

Choose a reason for hiding this comment

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

Hey, looking good! Do you think we can tie this to existing events rather than sending a new one though?
Also, can we maybe make the text stand out a little more? Maybe be bold and orange/green?

default-plugins/status-bar/src/main.rs Outdated
}

fn update(&mut self, event: Event) {
if let Event::ModeUpdate(mode_info) = event {
self.mode_info = mode_info;
dbg!("got event {:?}", &event);
Copy link
Member

Choose a reason for hiding this comment

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

oops :)

@tlinford
Copy link
Contributor Author

Hey, looking good! Do you think we can tie this to existing events rather than sending a new one though?

Not sure to be honest, out of the existing Event variants only Key seems to be close but imo doesn't really apply, specially for the event that signals that text was copied.
Do you think something along these lines this would be cleaner?

pub enum Event {
    ...,
    Action,
}

pub enum Action {
    InputReceived,
    SelectionCopiedToClipboard,
}

Action is maybe a little too generic/vague?

Also, can we maybe make the text stand out a little more? Maybe be bold and orange/green?
No problem.

@imsnif
Copy link
Member

imsnif commented Aug 19, 2021

Not sure to be honest, out of the existing Event variants only Key seems to be close but imo doesn't really apply, specially for the event that signals that text was copied.

What I was thinking is that instead of using InputReceived we could add the relevant line here: https://github.com/zellij-org/zellij/blob/main/zellij-server/src/route.rs#L26

Like - any action we get is essentially an "InputReceived", right? So before the match we could clear the "copied" line from the status bar. What do you think?

@imsnif
Copy link
Member

imsnif commented Aug 19, 2021

P.S. - sorry for delaying so long on replying :)

@tlinford
Copy link
Contributor Author

Hi @imsnif, I've removed the InputReceived ClientToServerMsg, using existing Actions to know when to send the Event to the plugins.
Also made the hint bold green, much better.

@imsnif
Copy link
Member

imsnif commented Aug 23, 2021

Looks great! Thanks for this cool feature :)

@imsnif imsnif merged commit 94f20cf into zellij-org:main Aug 23, 2021
This was referenced Nov 25, 2021
Open
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@imsnif imsnif imsnif requested changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Feature: indicate to the user when text is copied to the clipboard
2 participants
@tlinford @imsnif