patternfly-elements icon indicating copy to clipboard operation
patternfly-elements copied to clipboard

feat(timestamp): pfe-datetime to pfe-timestamp for 1:1

Open kylebuch8 opened this issue 3 years ago • 7 comments

<Pull request description>

Converting pfe-datetime to pfe-timestamp.

Related issues

  • (#2121) converting pfe-datetime to pfe-timestamp

Preview

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

What has changed and why

Converting pfe-datetime to pfe-timestamp.

Browser requirements

Your component should work in all of the following environments:

  • [ ] Latest 2 versions of Edge
  • [ ] 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 [email protected] mailing list!

kylebuch8 avatar Sep 09 '22 13:09 kylebuch8

🦋 Changeset detected

Latest commit: 8f725c777194bfa429b911ba827c5d9f1abc06bf

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 3 packages
Name Type
@patternfly/pfe-tooltip Minor
@patternfly/pfe-timestamp Minor
@patternfly/pfe-tools Minor

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

changeset-bot[bot] avatar Sep 09 '22 13:09 changeset-bot[bot]

Deploy Preview for patternfly-elements ready!

Name Link
Latest commit f7ee0f1a6237a0a59f74a77f8b5ec038a5f2a80b
Deploy Preview https://deploy-preview-2147--patternfly-elements.netlify.app/

To edit notification comments on pull requests, go to your Netlify site settings.

github-actions[bot] avatar Sep 09 '22 14:09 github-actions[bot]

One issue we're going to need to work through is adding a dashed underline to text when the pfe-timestamp component is surrounded by pfe-tooltip. I have an idea on the approach but I'm not sure if I like it.

connectedCallback() {
  super.connectedCallback();
  
  if (this.parentElement && this.parentElement.tagName.toLowerCase() === 'pfe-tooltip') {
    this.classList.add("has-tooltip");
  }
}

Something like that.

kylebuch8 avatar Sep 11 '22 14:09 kylebuch8

One issue we're going to need to work through is adding a dashed underline to text when the pfe-timestamp component is surrounded by pfe-tooltip. I have an idea on the approach but I'm not sure if I like it.

connectedCallback() {
  super.connectedCallback();
  
  if (this.parentElement && this.parentElement.tagName.toLowerCase() === 'pfe-tooltip') {
    this.classList.add("has-tooltip");
  }
}

Something like that.

how about this in pfe-tooltip styles:

:host {
  --_timestamp-text-decoration: underline dashed 1px 4px;
}

bennypowers avatar Sep 28 '22 09:09 bennypowers

how about this in pfe-tooltip styles:

:host {
  --_timestamp-text-decoration: underline dashed 1px 4px;
}

To prevent altering "private" variable names from outside of the component I wonder if we can use the magical space toggles trick 😀

⚠️ WARNING: EXTREME PSUEDO CODE AHEAD

:host {
  --pfe-tooltip-enabled: ;
}

heyMP avatar Oct 11 '22 17:10 heyMP

this is very cool and also very confusing and i'm having a hard time finding the benefit delta between sprinkling media queries throughout a stylesheet and sprinkling comments with links to a blog post in all the same spots

bennypowers avatar Oct 11 '22 17:10 bennypowers

@bennypowers yeah I agree. Something to keep in the back of the mind.

heyMP avatar Oct 11 '22 20:10 heyMP