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

Classifier: Clean up TaskInput styling #6532

Merged
merged 3 commits into from
Dec 12, 2024
Merged

Conversation

goplayoutside3
Copy link
Contributor

Package

lib-classifier

Linked Issue and/or Talk Post

Follow-up on #6511. The task input's lost their min-height in that PR, so I've refactored the shared component's styling in this PR.

Describe your changes

  • Refactor TaskInput's components with more intuitive CSS
  • Remove enzyme and defaultProps where possible

How to Review

  • Compare the lib-classifier Storybook on this branch to the deployed Storybook. The Tasks category should look almost identical.
    • The taskInput label and status used to be two different font sizes so I standardized them. Therefore, the drawing tasks and volumetric task in Storybook will look slightly different on this branch.
    • I added stories specifically for different types of TaskInput labels. One that isn't used yet in an FEM project yet is markdown with an image and label. Galaxy Zoo does this and you can preview it on this branch's lib-classifier at https://local.zooniverse.org:8080/?project=5733&env=production

Checklist

PR Creator - Please cater the checklist to fit the review needed for your code changes.
PR Reviewer - Use the checklist during your review. Each point should be checkmarked or discussed before PR approval.

General

  • Tests are passing locally and on Github
  • Documentation is up to date and changelog has been updated if appropriate
  • You can yarn panic && yarn bootstrap or docker-compose up --build and FEM works as expected
  • FEM works in all major desktop browsers: Firefox, Chrome, Edge, Safari (Use Browserstack account as needed)

General UX

Example Staging Project: i-fancy-cats

  • All pages of a FEM project load: Home Page, Classify Page, and About Pages
  • Can submit a classification
  • Can sign-in and sign-out
  • The component is accessible

Refactoring

  • The PR creator has described the reason for refactoring
  • The refactored component(s) continue to work as expected

@goplayoutside3 goplayoutside3 added the refactor Refactoring existing code label Dec 10, 2024
<Markdownz>{status}</Markdownz>
<StyledInputStatus>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This status is always strings of count and label provided by the tool models. No need for markdown.

Comment on lines -49 to +50
margin-top: .5em;
margin-bottom: .5em;
align-items: center;
min-height: 45px;
margin: 8px 0;
Copy link
Contributor Author

@goplayoutside3 goplayoutside3 Dec 10, 2024

Choose a reason for hiding this comment

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

Margin and padding are most useful as rigid units like px, not em or rem.

<StyledText>
<Markdownz inline >{label}</Markdownz>
<Markdownz inline>{label}</Markdownz>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This Markdownz is needed because the label is controlled by project teams via the lab.

@coveralls
Copy link

coveralls commented Dec 10, 2024

Coverage Status

coverage: 77.39% (+0.03%) from 77.363%
when pulling 2d69e05 on task-input-label
into 57c52de on master.

Copy link
Contributor

@kieftrav kieftrav left a comment

Choose a reason for hiding this comment

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

Love the cleanup - LGTM!

@github-actions github-actions bot added the approved This PR is approved for merging label Dec 11, 2024
@goplayoutside3 goplayoutside3 merged commit 3e8f2ee into master Dec 12, 2024
8 checks passed
@goplayoutside3 goplayoutside3 deleted the task-input-label branch December 12, 2024 00:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
approved This PR is approved for merging refactor Refactoring existing code
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants