Skip to content
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

Overhaul content and appearance of Guide screen (checklist) #152

Merged
merged 16 commits into from
Aug 19, 2024

Conversation

yxu-lanl
Copy link
Collaborator

#140

Checklist/Guide page
Add new fieldwork .md.ts files, fieldworkChecklist.ts, and public/assets/images/field-ethics.png
Mark the nested list item with 'NESTEDLISTITEM' in the .md.ts files since react-markdown couldn't handle nested list
Update Checklist.tsx: remove IonAccordion and IonPopover, use SectionHeader, use different icon for the nested list item
Update Checklist.module.css: place checkMark icon to the left-top of IonItem
Delete soilPackage files

@pkalita-lbl
Copy link
Collaborator

I think the changes look good. I just wanted to confirm with @jkelliher-github: do we need to be worried about including the GSA Field Ethics image here? I like it, but I don't want to step on anyone's toes.

@eecavanna
Copy link
Collaborator

eecavanna commented Aug 15, 2024

Looks good, @yxu-lanl!

I share @pkalita-lbl's concern about using that image.

I left some feedback about some of the checklist items. I don't know who the original author of those was. I'd be OK merging those items in as is, and then fixing them in a separate PR (so as not to hold up introducing this updated checklist UI over some content issues).

I do want to wait for the final word about using that image, before merging this PR in, though. Note: We could make our own colorful image (with our own tips); and also link out to their image, hosted by them.

@eecavanna
Copy link
Collaborator

eecavanna commented Aug 19, 2024

Hi @yxu-lanl, here's a link to the ticket I created about using that image: #158

My understanding from today's squad meeting was that we would move forward with this plan:

  • 1. Create ticket about confirming we can use that image (@eecavanna)
  • 2. Remove image from this PR branch for now (@yxu-lanl)
  • 3. Commit the suggestions on this PR branch, which are spelling/grammar-related (@yxu-lanl)
  • 4. Request a follow-up review of this PR branch (@yxu-lanl)

@eecavanna eecavanna changed the title 140 update checklist content Overhaul content and appearance of Guide screen (checklist) Aug 19, 2024
@eecavanna eecavanna self-requested a review August 19, 2024 18:52
Copy link
Collaborator

@eecavanna eecavanna left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for making those changes. I'm comfortable with this branch being merged into main now.

Copy link
Collaborator

@pkalita-lbl pkalita-lbl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks Yan!

@yxu-lanl yxu-lanl merged commit 58ff148 into main Aug 19, 2024
1 check passed
@yxu-lanl yxu-lanl deleted the 140-update-checklist-content branch August 19, 2024 19:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants