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

feat(progress): update linear progres props and add variants for circular #15385

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

didimmova
Copy link
Contributor

@didimmova didimmova commented Feb 20, 2025

Test with this theming package

Additional information (check all that apply):

  • Bug fix
  • New functionality
  • Documentation
  • Demos
  • CI/CD

Checklist:

  • All relevant tags have been applied to this PR
  • This PR includes unit tests covering all the new code (test guidelines)
  • This PR includes API docs for newly added methods/properties (api docs guidelines)
  • This PR includes feature/README.MD updates for the feature docs
  • This PR includes general feature table updates in the root README.MD
  • This PR includes CHANGELOG.MD updates for newly added functionality
  • This PR contains breaking changes
  • This PR includes ng update migrations for the breaking changes (migrations guidelines)
  • This PR includes behavioral changes and the feature specification has been updated with them

@didimmova didimmova added the ❌ status: awaiting-test PRs awaiting manual verification label Feb 20, 2025
@didimmova didimmova marked this pull request as ready for review February 20, 2025 15:06
@didimmova didimmova linked an issue Feb 20, 2025 that may be closed by this pull request
@desig9stein
Copy link
Contributor

@didimmova Modifying the --fill-color-default variable does not affect the progress bar's color. Additionally, while it may not be the appropriate place to address this, I noticed an issue with the gradient—its colors are reversed, with the start color appearing at the end and the end color at the beginning.

$progress-circle-color-start: map.get($theme, 'progress-circle-color');
$progress-circle-color-end: map.get($theme, 'progress-circle-color');
$fill-color-default-start: map.get($theme, 'fill-color-default');
$fill-color-default-end: map.get($theme, 'fill-color-default');
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm not sure if renaming progress-circle-color-start is the best approach, as it now gives the impression that I have variables, such as fill-color-info-start, which do not exist.

}

@return extend($theme, (
name: $name,
base-circle-color: $base-circle-color,
progress-circle-color-start: $progress-circle-color-start,
progress-circle-color-end: $progress-circle-color-end,
fill-color-default-start: $fill-color-default-start,
Copy link
Contributor

Choose a reason for hiding this comment

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

The fill-color-default should also be here, otherwise the user can't use it

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think the user uses it anyway, as in the beginning it's separated into two colors which form a gradient, no matter if it's one or two colors so the progress-circle-color(before) itself never gets used in the theme.

@didimmova
Copy link
Contributor Author

didimmova commented Feb 20, 2025

@didimmova Modifying the --fill-color-default variable does not affect the progress bar's color. Additionally, while it may not be the appropriate place to address this, I noticed an issue with the gradient—its colors are reversed, with the start color appearing at the end and the end color at the beginning.

I guess that's a whole new refactoring issue as it didn't work before also. And we're just adding variants and properties in this one.

@desig9stein
Copy link
Contributor

@didimmova Modifying the --fill-color-default variable does not affect the progress bar's color. Additionally, while it may not be the appropriate place to address this, I noticed an issue with the gradient—its colors are reversed, with the start color appearing at the end and the end color at the beginning.

I guess that's a whole new refactoring issue as it didn't work before also. And we're just adding variants and properties in this one.

@simeonoff should I log separate issues for this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Progress Bars] - Add more properties
2 participants