Fix minor style issues and platform component#178
Conversation
❌ Deploy Preview for open-devfounders failed.
|
❌ Deploy Preview for open-crowd-prod failed.
|
mariobalca
left a comment
There was a problem hiding this comment.
Left some comments/questions, that I'd like for us to discuss before merging.
Other than that, awesome work as always!
| </el-button> | ||
| </div> | ||
| <div class="app-list-table panel"> | ||
| <div class="user-list-table panel"> |
There was a problem hiding this comment.
The idea with app-list-table was to have a generic CSS class.
Can we not use it on this user-list-table.vue component?
There was a problem hiding this comment.
Thanks for the heads up. I didn't remove it, I used the app-list-table class but added an extra class to prevent the style I wanted.
I added non-clickable class; if present, the row on hover won't have the cursor pointer
| <div class="flex gap-3 items-center"> | ||
| <app-platform | ||
| :platform="platform" | ||
| :platform-name="`${platform}-icon`" |
There was a problem hiding this comment.
Shouldn't platform-name be a readable name like GitHub or StackOverflow?
There was a problem hiding this comment.
Since I was using platform-name for both the tracking event names and the alt property of the images it didn't have to be readable. But to prevent errors and confusion I removed this prop and added one for the track events:
trackEventName, if passed we will track the clicks
For the alt property, I'm now just getting the name value of the integrations json from the platform value
|
Perfect! Thanks for addressing my comments |
Changes proposed ✍️
Screenshots (front-end changes only)
Checklist ✅
type:feature 🚀,type:enhancement ✨,type:bug 🐞, ortype:documentation 📜.frontend/.env.distbackend/.env.dist,backend/.env.dist.staging,backend/.env.dist.staging.