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

refactor: column settings / color picker #4589

Merged
merged 7 commits into from
Oct 28, 2024
Merged

Conversation

Schwehn42
Copy link
Collaborator

Description

This PR refactors and improves on some issues with the current ColumnSettings and ColorPicker components, in order to make it usable within other components.

Separation of concerns

Previously, ColorPicker would dispatch the function to change the column color itself.
Since in other implementations, other things should happen upon selection, this has been moved to the parent component; a function with the selected color is propagated.

Unified rendering

Since the ColorPicker had no state whether it was open or closed, the closed version would be rendered in the parent component.
This lead to CSS classes being mixed.
Now, also the closed color picker is rendered within the color picker itself, so no CSS has to be imported by the parent component

Other changes

While we're at it, some other changes were made in an attempt to improve readability.

Changelog

  • Separate logic to select a color in the parent component; a function with the selected color is propagated by ColorPicker.
  • Closed ColorPicker is now also rendered within the component itself
    • open state is passed to it for this
  • Rename CSS classes to match ColorPicker
  • Simplify props for ColorPicker and ColumnSettings
    • pass column as Column type instead of separate property
    • Remove duplicate close functions which had the same purpose
  • Remove unneeded optional chaining and nullish coalescing
  • Cleanup imports
  • Rename MiniMenu prop icon to element to generalize purpose

Checklist

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas

@Schwehn42 Schwehn42 added the Review Needed This pull request is ready for review label Oct 25, 2024
@Schwehn42 Schwehn42 self-assigned this Oct 25, 2024
Copy link

The deployment to the dev cluster was successful. You can find the deployment here: https://4589.development.scrumlr.fra.ics.inovex.io
This deployment is only for testing purposes and will be deleted after 1 week.
To redeploy rerun the workflow.
DO NOT STORE IMPORTANT DATA ON THIS DEPLOYMENT

Deployed Images
  • ghcr.io/inovex/scrumlr.io/scrumlr-frontend:sha-9dfb31a

  • ghcr.io/inovex/scrumlr.io/scrumlr-server:sha-9dfb31a

Copy link
Collaborator

@laila-rin laila-rin left a comment

Choose a reason for hiding this comment

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

lgtm, thanks! :)

@Schwehn42 Schwehn42 removed the Review Needed This pull request is ready for review label Oct 28, 2024
@Schwehn42 Schwehn42 added this pull request to the merge queue Oct 28, 2024
Merged via the queue into main with commit c94b23d Oct 28, 2024
11 of 12 checks passed
@Schwehn42 Schwehn42 deleted the js/refactor-color-picker branch October 28, 2024 09:07
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.

2 participants