Skip to content

Conversation

@OS-pedrolourenco
Copy link
Contributor

@OS-pedrolourenco OS-pedrolourenco commented Feb 11, 2025

Issue number: internal


What is the current behavior?

ion-progress-bar does not currently have any custom styling for the ionic theme.

What is the new behavior?

According to the design, the following styling changes were made:

  • Added a new shape prop which controls the shape of the progress bar
  • Changed the color of the unfilled part of the progress bar and the bar height to design token values
  • Adding a new testing page and screenshot tests

Does this introduce a breaking change?

  • Yes
  • No

Other information

@vercel
Copy link

vercel bot commented Feb 11, 2025

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
ionic-framework ✅ Ready (Inspect) Visit Preview 💬 Add feedback Feb 13, 2025 1:20pm

@github-actions github-actions bot added package: core @ionic/core package package: angular @ionic/angular package package: vue @ionic/vue package labels Feb 11, 2025
@OS-pedrolourenco OS-pedrolourenco marked this pull request as ready for review February 11, 2025 17:01
@OS-pedrolourenco OS-pedrolourenco requested a review from a team as a code owner February 11, 2025 17:01
@OS-pedrolourenco OS-pedrolourenco requested review from rugoncalves and thetaPC and removed request for rugoncalves February 11, 2025 17:01
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

As mentioned in other PRs, code needs to be reviewed after being copying and pasting and updated appropriately.

Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

Also we need to have a snapshot for basic. Make sure to update progress-bar/test/basic/progress-bar.e2e.ts to include ionic-md

- configs().forEach(({ title, screenshot, config }) => {
+ configs({ modes: ['ios', 'md', 'ionic-md'] }).forEach(({ title, screenshot, config }) => {

Copy link
Contributor

Choose a reason for hiding this comment

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

Delete the shape snapshots and re-generate them. Playwright isn't able to detect the background change to buffer. That's why we need to force a re-generation. Background shouldn't be white, but neutral 100.

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 have regenerated the screenshots but no changes were made in the regeneration. I think the bar background looks white in comparison with #ccc7c7 colour we set to the container background, which is more gray-ish, but in reality it's not white.

@thetaPC thetaPC changed the title feat(progress-bar): add new styling for the ionic theme feat(progress-bar): add styling for the ionic theme Feb 12, 2025
Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor

@thetaPC thetaPC left a comment

Choose a reason for hiding this comment

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

LGTM

@OS-pedrolourenco OS-pedrolourenco merged commit ae78967 into next Feb 13, 2025
47 checks passed
@OS-pedrolourenco OS-pedrolourenco deleted the ROU-11591 branch February 13, 2025 16:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

package: angular @ionic/angular package package: core @ionic/core package package: vue @ionic/vue package

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants