-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: badge in progress content component #5618
base: main
Are you sure you want to change the base?
Conversation
christian14b
commented
Oct 25, 2024
src/composables/useBadgeModal.js
Outdated
@@ -30,6 +30,12 @@ export const ID_US_ECONOMIC_EQUALITY = 'us-economic-equality'; | |||
export const ID_CLIMATE_ACTION = 'climate-action'; | |||
export const ID_REFUGEE_EQUALITY = 'refugee-equality'; | |||
export const ID_BASIC_NEEDS = 'basic-needs'; | |||
// TODO: Use the LoanChannelQueryMap to get the correct filters |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is a ticket logged for this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this as a note because I wasn't sure if we have already handled this (the query params needed to redirect users to lend/filter
), but I'll create a follow-up ticket if it's not
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The query map will work if our achievements directly match existing channels in that map file. Do our achievements as defined match existing channels?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most of them, the problem is the OR
conditions in the Badge breakdowns
spreadsheet I think... Seems like only Climate actions
and Basic needs
will need to be handled differently
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, I don't think we will need the query map to replace this, we need specifics query params for Climate actions
and Basic needs
class="tw-pt-1 tw-pb-1.5 tw-flex tw-flex-col md:tw-flex-row tw-items-center tw-justify-left tw-gap-3" | ||
> | ||
<BadgeContainer :status="BADGE_IN_PROGRESS" :shape="getBadgeShape()" class="tw-z-1"> | ||
<img |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The dashed border is showing up inside the badge icon. It should show around the outside. Is this because of the maxheight override? Also, it would be helpful to add BadgeModal
stories to cover the 5 different badge use cases, to ensure the badge icon looks correctly for each.
Related, this version of the BadgeModal
requires there to be modal content to be complete compared to the mocks (modal without title, back button, etc.). Is that something you see as a follow-up ticket?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good regarding the follow-up ticket. Please include a link to the mocks and bullets for the specific requirements in the mocks that are still needed.
}; | ||
|
||
const badgeName = computed(() => props.badge?.fields?.challengeName ?? ''); | ||
const badgeImage = computed(() => props.badge?.fields?.badgeImage?.fields?.file?.url ?? ''); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This will need to change once we update to use all badge icons, this needs to be the badge of the current level. I'm creating utils to get the correct tier. Please add a TODO
here to use the icon of the correct in-progress badge level.