www
/
cs-2022-class-profile
16
0
Fork 0
Code Issues 11 Pull Requests 1 Projects Releases Wiki Activity

Component Switcher #169

Merged
r2shuai merged 7 commits from richardshuai/component-switcher into main 2023-02-07 13:28:18 -05:00
Conversation 1 Commits 7 Files Changed 4 +204 -14
r2shuai commented 2023-02-04 21:11:35 -05:00
Member
Copy Link

This is specifically for Issue #142.

Created a new PR for component switcher changes.

https://richardshuai-component-switcher-csc-class-pr-snedadah.k8s.csclub.cloud/samplePage/

This is specifically for Issue #142. Created a new PR for component switcher changes. https://richardshuai-component-switcher-csc-class-pr-snedadah.k8s.csclub.cloud/samplePage/
r2shuai added 3 commits 2023-02-04 21:11:37 -05:00
continuous-integration/drone/push Build is failing Details
a4a012a7a6 Creating component switcher
continuous-integration/drone/push Build is failing Details
408896098e component switcher and attempt to style
continuous-integration/drone/push Build is passing Details
e191bdf40b component switcher changes
r2shuai requested review from e26chiu 2023-02-04 21:11:50 -05:00
r2shuai added 1 commit 2023-02-04 21:12:15 -05:00
continuous-integration/drone/push Build is passing Details
b982a1103f Merge branch 'main' into richardshuai/component-switcher
snedadah reviewed 2023-02-05 13:48:19 -05:00
components/ComponentSwitcher.module.css Outdated
@ -0,0 +23,4 @@
}
.btn:hover {
--button-color: #CA634D;
snedadah commented 2023-02-05 13:48:19 -05:00
Owner
Copy Link

I think we should choose differnet colors for the hover vs selected states.

Also, if you go here: https://richardshuai-component-switcher-csc-class-pr-snedadah.k8s.csclub.cloud/playground/ you can see some of the different colors that we already have as variables, it might be better to reuse one of those if possible? (if none of them look good, thats totally fine to use a new one then!, but maybe try those as well!)

I think we should choose differnet colors for the hover vs selected states. Also, if you go here: https://richardshuai-component-switcher-csc-class-pr-snedadah.k8s.csclub.cloud/playground/ you can see some of the different colors that we already have as variables, it might be better to reuse one of those if possible? (if none of them look good, thats totally fine to use a new one then!, but maybe try those as well!)
r2shuai commented 2023-02-05 21:01:01 -05:00
Author
Member
Copy Link

I chose the colors from the global theme. I can definitely change the hover to a different color. Let me know if I need to change the button colors too :)

I chose the colors from the global theme. I can definitely change the hover to a different color. Let me know if I need to change the button colors too :)
snedadah reviewed 2023-02-05 13:51:02 -05:00
components/ComponentSwitcher.tsx
@ -0,0 +34,4 @@
key={idx}
className={`${styles.btn} ${
// eslint-disable-next-line @typescript-eslint/restrict-template-expressions
selectedButton === buttonName && styles.selectedBtn
snedadah commented 2023-02-05 13:51:02 -05:00
Owner
Copy Link

I'm not sure if this is exactly correct? Doesn't this just evalute to 'true' or false?

I think what you meant to do is

selectedButton === buttonName ? styles.selectedBtn : ""

Thats the ternary operator which is basically an inline if statment.

I'm not sure if this is exactly correct? Doesn't this just evalute to 'true' or false? I think what you meant to do is `selectedButton === buttonName ? styles.selectedBtn : ""` Thats the ternary operator which is basically an inline if statment.
r2shuai commented 2023-02-05 21:02:01 -05:00
Author
Member
Copy Link

I think this expression works like if (selectedButton === buttonName) styles.selectedBtn in JSX. I will change to the ternary to make it more readble though!

I think this expression works like `if (selectedButton === buttonName) styles.selectedBtn` in JSX. I will change to the ternary to make it more readble though!
snedadah commented 2023-02-06 21:59:21 -05:00
Owner
Copy Link

And you don't need the @ts-ignore anymore too!

And you don't need the @ts-ignore anymore too!
snedadah reviewed 2023-02-05 13:54:25 -05:00
snedadah left a comment
Owner
Copy Link

On mobile, the buttons are very close together, and also the left most button is cutoff a bit.

image

On mobile, the buttons are very close together, and also the left most button is cutoff a bit. ![image](/attachments/7889118a-b2b9-4e31-8a9f-02b03c7eb598)
image.png
75 KiB
snedadah reviewed 2023-02-05 14:03:15 -05:00
snedadah left a comment
Owner
Copy Link

With moving it outside of the intenral wrapper and also changing it to display inline:

image

Maybe thats one solution (not saying this is the best one, just one option maybe, though this has other issues)

With moving it outside of the intenral wrapper and also changing it to display inline: ![image](/attachments/da81428f-592a-4cbd-a6fb-3c57285cb87e) Maybe thats one solution (not saying this is the best one, just one option maybe, though this has other issues)
components/ComponentSwitcher.module.css Outdated
@ -0,0 +31,4 @@
}
.btnContainer {
display: flex;
snedadah commented 2023-02-05 14:00:45 -05:00
Owner
Copy Link

To fix the container issues, maybe a changing this to "display: inline" is better?
Not sure if this is the best solution, but it seems to work better.

To fix the container issues, maybe a changing this to "display: inline" is better? Not sure if this is the best solution, but it seems to work better.
r2shuai commented 2023-02-05 21:03:06 -05:00
Author
Member
Copy Link

Right I think inline definitely looks better. But I also tried block, and it works well too!

Right I think inline definitely looks better. But I also tried block, and it works well too!
r2shuai added 1 commit 2023-02-05 21:19:30 -05:00
continuous-integration/drone/push Build is passing Details
94435d29f1 Button change for component switcher
r2shuai commented 2023-02-05 21:21:46 -05:00
Author
Member
Copy Link

Please review the new commit, thank you :)

It should be displaying normally on mobile now.

Please review the new commit, thank you :) It should be displaying normally on mobile now.
snedadah added 1 commit 2023-02-06 23:05:32 -05:00
continuous-integration/drone/push Build is passing Details
75e61f4a1d Changed CSS colors
snedadah added 1 commit 2023-02-06 23:06:05 -05:00
continuous-integration/drone/push Build is passing Details
55abe0e841 Fixed extra space
snedadah approved these changes 2023-02-06 23:30:14 -05:00
snedadah left a comment
Owner
Copy Link

Looks good to me, nice work!!

I just quickly changed the color to something I think looks better given our color scheme, and I also changed the px values to `calc(..rem/16) which is something we do which I can explain more later. Thanks for working on this!

Looks good to me, nice work!! I just quickly changed the color to something I think looks better given our color scheme, and I also changed the px values to `calc(..rem/16) which is something we do which I can explain more later. Thanks for working on this!
r2shuai merged commit ff27c3cbc0 into main 2023-02-07 13:28:18 -05:00
r2shuai deleted branch richardshuai/component-switcher 2023-02-07 13:28:19 -05:00
r2shuai referenced this issue from a commit 2023-02-07 13:28:20 -05:00
Component Switcher (#169)
Sign in to join this conversation.
Reviewers
No reviewers
e26chiu
snedadah
Labels
Clear labels   
Bug

   
Component

   
Config

   
Good First Issue

   
Low-Priority

   
Page
No Label
Bug
Component
Config
Good First Issue
Low-Priority
Page
Milestone
Clear milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
No Assignees
2 Participants
Notifications
Due Date
The due date is invalid or out of range. Please use the format 'yyyy-mm-dd'.

No due date set.

Dependencies

No dependencies set.

Reference: www/cs-2022-class-profile#169
No description provided.
Delete Branch "richardshuai/component-switcher"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?