-
Notifications
You must be signed in to change notification settings - Fork 178
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
Enable priority checkbox for admin and participant #1562
Enable priority checkbox for admin and participant #1562
Conversation
@@ -1 +1,3 @@ | |||
*~ | |||
.history/ | |||
math/.cpcache/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We've been using separate .gitignore
files in each of the subdirectories, so I think we should stick with that pattern unless we have a broader conversation about switching to a single/main .gitignore
.
@@ -1 +1,3 @@ | |||
*~ | |||
.history/ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is .history
coming from? I think this can stay if it's something we expect lots of other devs to have. But if it's a detail of your particular dev setup, I think it makes sense for you to just put that in your ~/.gitignore
. If it stays though, it should have a comment about what it's for.
from { | ||
background-color: rgba(0, 0, 0, 0); | ||
} | ||
10% { | ||
background-color: rgb(202, 227, 255); | ||
} | ||
to { | ||
background-color: rgba(0, 0, 0, 0); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I actually like the compact view in cases like this.
#agreeButton, #disagreeButton, #passButton { | ||
#agreeButton, | ||
#disagreeButton, | ||
#passButton { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Again, prefer the more compact form here.
display: -webkit-box; /* OLD - iOS 6-, Safari 3.1-6 */ | ||
display: -moz-box; /* OLD - Firefox 19- (buggy but mostly works) */ | ||
display: -ms-flexbox; /* TWEENER - IE 10 */ | ||
display: -webkit-flex; /* NEW - Chrome */ | ||
display: flex; /* NEW, Spec - Opera 12.1, Firefox 20+ */ | ||
display: -webkit-box; /* OLD - iOS 6-, Safari 3.1-6 */ | ||
display: -moz-box; /* OLD - Firefox 19- (buggy but mostly works) */ | ||
display: -ms-flexbox; /* TWEENER - IE 10 */ | ||
display: -webkit-flex; /* NEW - Chrome */ | ||
display: flex; /* NEW, Spec - Opera 12.1, Firefox 20+ */ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Prefer the original here
Thanks so much for submitting this @chena11356! I'm going to stop reviewing, because it's really challenging to have to wade through all of the whitespace changes. We can discuss a separate issue/PR for this work if you like. Though I have to say, I'm somewhat disinclined from considering something like this across the repo right now with a lot of work going on in various forks that we're trying to get merged. If we do this, we should coordinate with @colinmegill about regarding styleguides and linter settings. |
relates to #217 |
Update: for now we will hide the checkbox for the admin and manually switch it "on" for organizations/conversations as needed. |
Adding Cypress testing to this feature would be great also |
closing in favor of #1682 |
No description provided.