community-platform
community-platform copied to clipboard
fix: used theme specific icons for fixing-fashion
PR Checklist
- [x] - Commit messages are descriptive, it will be used in our Release Notes
PR Type
- [x] Bug fix (non-breaking change which fixes an issue)
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing functionality to change)
- [ ] Developer experience (improves developer workflows for contributing to the project)
Description
Theme specific icons for fixing-fashion
Git Issues
Closes #1948
Screenshots/Videos
What happens next?
Thanks for the contribution! We try to make sure all PRs are reviewed ahead of a monthly dev call (first Monday of the month, open to all!).
If the PR is working as intended it'll be merged and included in the next platform release, if not changes will be requested and re-reviewed once updated.
If you need more immediate feedback you can try reaching out on Discord in the Community Platform development channel.
@davehakkens We don't have all the assets for every theme present, for example precious plastic assets are being used in project kamp as well. If we can have those assets then I can add theme specific checks for each of these assets.
Hey thanks I could provide the assets for (kamp) one so we can add the theming for all? Seems worth it to do it all the way.
Also if i see it correctly running your PR locally
The icon you use for Precious Plastic is this one
But it should be this one (without text around it, optimised for small screens)

Visit the preview URL for this PR (updated for commit e319b89):
https://onearmy-next--pr1978-fix-1948-mljeww6u.web.app
(expires Mon, 07 Nov 2022 08:56:37 GMT)
🔥 via Firebase Hosting GitHub Action 🌎
@davehakkens sure let me have it and I'll add it for all 😃
OK cool. So here are the .svg in a zip Icons PK.zip For large and small display size.
But I noticed the names in the themes repository are a bit mixed. So i've added an image that gives the overview of how it should be.

@davehakkens can you check now? Let me know if you need any changes.
Nice. Just curious how do you test this one with all the different themes, locally? Annd looks like a test is failing @asheerrizvi?
Hey @davehakkens, used this theme select element to switch themes on local. I think this is visible when you are running it in dev mode:
I'll check out the test tomorrow I suppose.
any luck on the test @asheerrizvi?
Hey @davehakkens, I'll push it tomorrow. Was a bit busy these last few days.
@davehakkens I have fixed the tests. However, I think we will need verified member assets for each theme as well. Currently for verified member the only asset we have is based on Precious Plastic member icon. Should we do it now only or come back to it later?
Visit the preview URL for this PR (updated for commit 56333d4):
https://onearmy-next--pr1978-fix-1948-mljeww6u.web.app
(expires Tue, 22 Nov 2022 11:52:30 GMT)
🔥 via Firebase Hosting GitHub Action 🌎
Sign: 6d65e4f8fee2f6ab2da0c1c3b85b8797d66afa59
We won't have verified members @asheerrizvi, it's only for spaces. We might have a verified FF and PK Space. But not anytime soon. So we could also leave it like this for now and add those later.
@davehakkens cool, can you have a look and see if this can be merged?
This one is hard to test locally because it contains different themes and some element can only be loaded if the data is there. For instance a Fixing Fashion Space and Project Kamp member dont exist in our dev database or emulator.
For now i'll merge it so it gets deloyed to all the devsites and we can test it! Which means it might become an iterative proces, hope thats ok :)
@davehakkens yep, no worries. It was difficult for me to test on local as well. Let me know if anything seems out of ordinary.
Test summary
Run details
| Project | onearmy-community-platform |
| Status | Passed |
| Commit | 56333d440a |
| Started | Oct 21, 2022 6:17 PM |
| Ended | Oct 21, 2022 6:20 PM |
| Duration | 02:45 💡 |
| OS | Linux Ubuntu - 20.04 |
| Browser | Chrome 106 |
View run in Cypress Dashboard ➡️
This comment has been generated by cypress-bot as a result of this project's GitHub integration settings. You can manage this integration in this project's settings in the Cypress Dashboard
Hmm seems to fail when deploying to all 3 instances. https://app.circleci.com/pipelines/github/ONEARMY/community-platform/3211/workflows/78509865-77a1-488b-9538-00e2fb3cb62c
error here

@davehakkens not sure why it is erroring out for these two files. Building and serving it locally seems to work and also the deploy previews were working earlier in the PR :/
Weirdly my IDE is not flagging for these modules as missing, it's able to import them:
I can also try importing these using a relative path to see if that works. But I am not sure why is this causing an error for these two files only while PlatformTheme has been imported in a similar manner in other files like Sprites.tsx, Workspace.tsx etc.
Got some info for you on this @asheerrizvi These are the relevant error messages:
ERROR in /home/circleci/project/src/modules/profile/index.ts
[tsl] ERROR in /home/circleci/project/src/modules/profile/index.ts(3,36)
TS2307: Cannot find module 'src/themes/types' or its corresponding type declarations.
ERROR in /home/circleci/project/src/modules/profile/SupportedProfileTypesFactory.ts
[tsl] ERROR in /home/circleci/project/src/modules/profile/SupportedProfileTypesFactory.ts(19,36)
TS2307: Cannot find module 'src/themes/types' or its corresponding type declarations.
The issue here is taking place during the build process for the serverless functions. It looks like the changes you have introduced here have changed the dependency tree which is no resulting in broken tests.
You should be able to run yarn workspace functions build to recreate the behaviour which is breaking in CI. This will help to determine the source of this breakage
@davehakkens awesome, was able to fix it with this. Here's the PR: #1993
Some things to note:
- Moved all profile related imports to the
..profile/types.tsfile - Updated all files which were importing these types from
..profile/index.tsto import them from..profile/types.tsfile.
cool thanks for the changes. Let's see 🤞
Just tested on devsites. In case you ever need them PP = https://dev.onearmy.world PK = https://dev.community.projectkamp.com FF = https://dev.community.fixing.fashion
Works all good, thanks a lot for the cleanup @asheerrizvi
:tada: This PR is included in version 1.29.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket:
Found a little bug on this one @asheerrizvi in profile/settings
The member icon is wrong in this overview.
It shows the small, not large one.

@davehakkens got you. Will push a fix tomorrow.
little reminder on this @asheerrizvi Bug is still there
@davehakkens ahh slipped my mind. Here you go: https://github.com/ONEARMY/community-platform/pull/2003
:tada: This issue has been resolved in version 1.31.0 :tada:
The release is available on GitHub release
Your semantic-release bot :package::rocket: