PolicyEngine / policyengine-app

PolicyEngine's free web app for computing the impact of public policy.
GNU Affero General Public License v3.0
35 stars 89 forks source link

Change Switch color to Blue #1576

Closed czhou578 closed 1 month ago

czhou578 commented 1 month ago

Description

Fixes #1531

Changes

Changes the color of the ant design switch components to (#2C6496) which is specified as "BLUE" in the colors.jsx file.

Screenshots

image

Tests

No extra tests have been added

czhou578 commented 1 month ago

@anth-volk , could you take a look at the BiPanel.jsx to see how it is being used? I have already applied the blue color to all the other files with switch elements. Thanks!

czhou578 commented 1 month ago

Thanks for clarifying what BiPanel.jsx was doing. I guess I'll just leave that file in there. In the Ant documentation, I could not find a specific property that exists which allows me to set the color of the switch element. By the way, I don't see the video that you mentioned in your comment...

anth-volk commented 1 month ago

Haha that would be because I didn't add it. Here it is:

https://github.com/PolicyEngine/policyengine-app/assets/14987227/5777541d-1a82-44c9-a1b6-ba981aca22a6

czhou578 commented 1 month ago

@anth-volk, I don't believe I can imperatively set the switch color for the PolicyRightSidebar. If a user checks the switch and then refreshes the page, then the state tracking the checked state (default value is false) would stay false, even though the switch is already checked. I'm thinking we can use the localStorage on the browser so that we can persist the checked state even through refreshes, but I'm not sure if that's acceptable. What do you think?

anth-volk commented 1 month ago

I'm thinking the best thing we could do is upgrade to Ant Design v5, then implement their context provider, as looking through their docs, I believe that actually sets style changes via CSS and does so in a more comprehensive way.

At the moment, the upgrade is being done in #1523 by a different contributor, though I think it's been about a week since they've worked on it. I think we could see if they continue, but if not, would you be willing to continue that down the road? Then perhaps this (#1576) could be resolved via the context provider.

czhou578 commented 1 month ago

Makes sense. I will be willing to work on that issue. Maybe I'll give the other contributor another 1-2 days to make additional changes? If nothing happens by then, I can start working on that.

anth-volk commented 1 month ago

Actually, why don't I reach out to him directly tomorrow morning?

anth-volk commented 1 month ago

Just got confirmation from @dotslashbit that he'd be happy for you to take on the upgrade, @czhou578. This is currently open as PR #1523. Could you push the requested changes to his fork branch so that we can utilize the same PR?

czhou578 commented 1 month ago

@anth-volk I have tried to push my changes to his fork branch, but it gives me this error:

image

anth-volk commented 1 month ago

Ah, I think you'd have to clone his fork, then push against the branch.

Instead, then, could you try just pulling his changes in, then dropping yours on top and opening a new PR? Then we can close out his.

anth-volk commented 1 month ago

Closing in light of #1584. In order to properly do this, we'll need to do the things mentioned in that PR first and migrate to Ant Design v5 first.