WIP: Creating component switcher #158
Closed
r2shuai
wants to merge 2 commits from richardshuai/component-switcher into main
snedadah
e26chiu
Loading…
Reference in new issue
There is no content yet.
Delete Branch 'richardshuai/component-switcher'
Deleting a branch is permanent. It CANNOT be undone. Continue?
This is specifically for Issue #142.
Work in progress and please check if this is what we want for the component switcher. Thanks!
Nice work!
Yep, this is what we are looking for. I reccomend you add it to the top of the SamplePage so you can view and test it as you develop. (Copying some of those graphs as well)
https://git.csclub.uwaterloo.ca/www/cs-2022-class-profile/src/branch/main/pages/samplePage.tsx
The hard part is going to be having to style it. Let me know if you need any help or run into issues!
The current issue is: if two graphs are of different types (for example, pie and line), they might have different height.
The solutions are: 1. wrap the graph around a container that has a set height, but might disrupt its aspect ratio; 2. keep the way it is but the position might shift.
Also, the lint failed because I am currently not using an assigned state. We can change this later if everything else is complete.
Let me know if I need to change anything. Thanks for reviewing :)
Hey @r2shuai ! Thanks for working on this issue, it's definitely more complex since we don't have the design for this component! In our case (or at least, in this term), the type of the graph shouldn't change (it should be the same for all terms! Have we considered trying to put the buttons above instead? How would that look?
Otherwise, here are the comments and questions I have for now! Good work so far!
.btn {--button-color: #E55F98;--border-color: #FEA0C8;Just a question, where did you choose these colours from? I think we should aim to reuse colours defined in the
_app.cssfile as much as possible instead of definining new colours. It's also better if you define the variable names in that file so that it will be easier to change these values later on (since they are all mostly in that file).These colors are taken directly from
_app.css. I believe they are the primary and secondary accent colour. Let me know if I need to change them :)--button-color: #CA634D;--border-color: #FFBC9F;background-color: var(--button-color);There should be perhaps a
cursor: pointerproperty to make the button seem clickable?buttonList,graphList,}: ComponentSwitcherProps) {const [selectedButton, setSelectedButton] = useState("1A");Instead of hardcoding this value, we could perhaps use
buttonList[0]?Ok I will change that!
Hey @e26chiu I tried to move the buttons to the top, but I think it looks better in the bottom in general (especially when there're a lot of them). You can try to see which one looks better and let me know if it needs to be changed!
Reviewers