-
Notifications
You must be signed in to change notification settings - Fork 23
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
Event UI: Question List/Queue #205
Conversation
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.
Looks good. Will take a closer look at the CSS after I review spec.
input: { | ||
['& fieldset']: { | ||
borderRadius: 9999 // rounded text field | ||
}, | ||
} |
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.
Huh, this is interesting. Did you find this recommended somewhere in the MUI docs?
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 found it on a Github issue on how to edit text fields of MUI components. Link here.
<SearchIcon /> | ||
</InputAdornment> | ||
), | ||
// TODO: add refresh action |
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.
Make a github issue for this and link it here for better tracking
app/client/src/features/events/Moderation/ManageQuestions/CurrentQuestionCard.tsx
Outdated
Show resolved
Hide resolved
left: '50%', | ||
transform: 'translateX(-50%)', | ||
background: '#F5C64F', | ||
color: 'white', |
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.
theme.palette.background.default (so we can support dark mode in the future)
@@ -0,0 +1,124 @@ | |||
/* eslint-disable @typescript-eslint/indent */ |
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.
Hmm, is there a reason we're moving current question card outside of manage questions?
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.
Ah, I see after looking at the figma design. Is the question carousel getting removed?
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 think so based on the Figma design, so I made that current question card component.
app/client/src/features/events/Questions/QuestionList/QuestionList.tsx
Outdated
Show resolved
Hide resolved
<ThumbUp color='disabled' /> | ||
</Badge> | ||
<CardContent className={classes.stats}> | ||
<ThumbUp fontSize='small' htmlColor='#8EAFFF' /> |
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.
Please use theme based colors.
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 tried to match the colors from the Figma, so I'll add it to the theme file.
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.
Don't forget that you can use one of the theme colors + lighten or darken. https://mui.com/customization/palette/#default-values for more details.
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.
It looks and works well. Just two minor changes I'd like you to make. I think the current question should be excluded from the previous tab and the order of previous questions should be reversed so the last question asked would be at the top and oldest would be at the bottom. I think it flows better that way.
Also if it's not too difficult, could you make the yellow nextQuestion tab stationary when reordering that question?
…vent-ui-questions
cd14371
to
d17c939
Compare
1. Please briefly explain what it is you coded
Restyled question list and queue based on Figma design.
Nodes referenced:
2. Please briefly explain your approach (new components added? modified? removed?)
Use a combination of Material UI components to restyle question list and queue. Modified some mutations to allow variables to be used in certain components (i.e. last name for question cards, enqueue/dequeue functionality for upcoming questions, etc.) Created a separate component for the current question to be displayed at the top of the event sidebar.
3. Any packages added/updated/removed
N/A
(Optional) 4. Any feature recommendations/insights/questions/comments/etc.?