Skip to content

Conversation

@heyMP
Copy link
Contributor

@heyMP heyMP commented Aug 30, 2021

The progress bar line is dynamically calculated based on the size of the items. Because of this, we need to add a resize event so let the progres-steps element know to recalculate the bar.

Related issues

Preview

Link(s) to demo page(s) where this element can be viewed:

What has changed and why

  • Resize event listener added
  • Removed min-width: fit-contents: so that the --pfe-progress-steps__item--size property works as expected.

Testing instructions

Demo Snippet

let progressStep = document.querySelector('pfe-progress-steps');
let item = progressStep.querySelector('[slot=title]').innerText = "aoisdnf sfosdf asdf oasf";
progressStep.style.setProperty('--pfe-progress-steps__item--size', '300px');
progressStep._build();

Browser requirements

Your component should work in all of the following environments:

  • Latest 2 versions of Edge
  • Internet Explorer 11 (should be useable, not pixel perfect)
  • Latest 2 versions of Firefox (one on Mac OS, one of Windows OS)
  • Firefox 78 (or latest version for Red Hat Enterprise Linux distribution)
  • Latest 2 versions of Chrome (one on Mac OS, one of Windows OS)
  • Latest 2 versions of Safari
  • Android mobile device (such as the Galaxy S9)
  • Apple mobile device (such as the iPhone X)
  • Apple tablet device (such as the iPhone Pro)

Ready-for-merge Checklist

  • Expected files: all files in this pull request are related to one request or issue (no stragglers or scope-creep).
  • Tests have been updated to cover these changes.
  • Browser testing passed.
  • Changelog updated.
  • Documentation (README.md, WHY.md, etc.) updated or added.
  • Link to the demo recording:
  • Approved by designer.

Merging

Please squash when merging and ensure your commit message uses conventional commit formatting.

Be sure to share your updates with the patternfly-elements-contribute@redhat.com mailing list!

@github-actions github-actions bot added functionality Functionality, typically pertaining to the JavaScript. styles An issue or PR pertaining only to CSS/Sass labels Aug 30, 2021
@netlify
Copy link

netlify bot commented Aug 30, 2021

✔️ Deploy Preview for patternfly-elements ready!

🔨 Explore the source changes: 298d72c

🔍 Inspect the deploy log: https://app.netlify.com/sites/patternfly-elements/deploys/61320fd502ac7800079cec22

😎 Browse the preview: https://deploy-preview-1763--patternfly-elements.netlify.app

@github-actions github-actions bot added the AT passed Automated testing has passed label Aug 30, 2021
@github-actions github-actions bot added the tests Related to testing label Aug 31, 2021
@heyMP heyMP changed the title [WIP] feat: added resize observer for lining up progress bar [WIP] fix: added resize observer for lining up progress bar Aug 31, 2021
@heyMP heyMP changed the title [WIP] fix: added resize observer for lining up progress bar fix: added resize observer for lining up progress bar Sep 1, 2021
Comment on lines 3 to 5
- [5393d18](https://github.com/patternfly/patternfly-elements/commit/5393d185fec3b62e78037a9835470fc15adae2b3) fix: Pfe-Primary-Detail - Fixing animation jank when expanding a section in compact mode
- [04ccc7d](https://github.com/patternfly/patternfly-elements/commit/04ccc7de323bd503227d972d61707e6acb8f0b89) docs: Adjust example code block for typography classes
- [974ab6f](https://github.com/patternfly/patternfly-elements/commit/974ab6f1ab4047d4e94007d64a31e5a0cddf9b7a) fix: typos in package.json files
Copy link
Contributor Author

Choose a reason for hiding this comment

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

fixed up the commit logs.

:host(:not([vertical])) {
width: pfe-local(size, $region: item);
min-width: fit-content;
text-align: center;
Copy link
Contributor Author

Choose a reason for hiding this comment

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

The step items only line up correctly if the width is explicitly set. fit-content was messing with it.


constructor() {
super(PfeProgressSteps, { type: PfeProgressSteps.PfeType });
this._resizeObserver = new ResizeObserver(this._build.bind(this));
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add an IE11 check before this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added check.

super.connectedCallback();
this._build();
// this will call _build initially and estabilish a resize observer for this element
this._resizeObserver.observe(this);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we add an IE11 check before this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

checking for presence of resizeObserver first. If not, just build it one time. If it's IE11, then the _build command will catch it.

disconnectedCallback() {}
disconnectedCallback() {
super.connectedCallback();
this._resizeObserver.disconnect();
Copy link
Contributor

Choose a reason for hiding this comment

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

Add some logic to only disconnect if this._resizeObserver is present.

Copy link
Contributor

@kylebuch8 kylebuch8 left a comment

Choose a reason for hiding this comment

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

Leather Gloves Touch Mulch

@kylebuch8 kylebuch8 merged commit f727065 into master Sep 3, 2021
@kylebuch8 kylebuch8 deleted the issue-1710-progress-steps-progress-bar branch September 3, 2021 12:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

AT passed Automated testing has passed functionality Functionality, typically pertaining to the JavaScript. styles An issue or PR pertaining only to CSS/Sass tests Related to testing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants