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

Unable to make initial assignment of Action to user other than self #326

Closed
jordanh opened this issue Oct 4, 2016 · 5 comments
Closed

Comments

@jordanh
Copy link
Contributor

jordanh commented Oct 4, 2016

Issue - Bug

While creating an Action during a meeting, if you first click on the avatar in the card footer to try and reassign the action without having saved any action text, the action card will disappear.

@jordanh jordanh added the bug label Oct 4, 2016
@jordanh jordanh added this to the Epic 1: MVTP (Alpha) milestone Oct 4, 2016
@mattkrick
Copy link
Member

To fix this, we'll have to move the onBlur to the OutcomeCard instead of the OutcomeCardTextArea. Unfortunately, the card is a div & we know that tabIndex and react don't play well, so we should probably move it to redux & turn it into component number 2! will get this coded up tonight, this should be the last bugfix for the agenda-fix PR

@mattkrick
Copy link
Member

oh man this is a tricky one to the _The Right Way_™.

I could wrap the update function in a setTimeout to call it on the next tick, allowing the active = false to propagate to the update function, but historically, that is almost always the wrong way.
I could listen for a blur on the text area & as long as the click isn't on the card, I could call it, but then that takes away things like hitting the tab key.
When a textarea gets focus, i could add an event listener to the document, and when that click occurs outside of the card, then fire update and remove the event listener. That doesn't handle the tab key though.

For that, we'd have to say if the text area gets blurred, then we want to check if any child of the card has focus (assuming the footer has actually buttons or a tags, or we use the tabIndex trick & hope it works in react)

@mattkrick
Copy link
Member

This brings about an interesting case for when to use the NullCard.
Currently, we use NullCard if the current user is the owner of the project AND the content is falsy.

If we still wanted to use the NullCard, we'd have to include a createdBy field and show nullcard when the project.createdBy === current user && !content. Alternatively, we could do away with the NullCard & just show the card with matt is editing... in the header. I like this idea better because it's what we'll do for the meetings when we use per-keystroke updates, so the look will be more homogenous. Also, it includes the same info.
Thoughts?

@jordanh
Copy link
Contributor Author

jordanh commented Oct 8, 2016

I like going with plain matt is editing...; it's definitely more furture-forward!

@mattkrick
Copy link
Member

OK, game plan is to:

  • bring back NullCard
  • add a createdBy field on the server (until we implement optimistic UI)
  • show null card if createdBy is not your user name && !content

@jordanh jordanh closed this as completed Oct 11, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants