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

feat(tabs): PF1:1

Open zeroedin opened this issue 3 years ago • 7 comments

What I did

TODO:

  • [x] break out base classes for inheritance
  • [x] implement overflow styles
  • [x] implement inset styles
  • [x] implement icon tab styles
  • [x] implement filled styles
  • [x] fix spec tests
  • [x] improve docs (add CSS props jsdoc comments)
  • [x] add changeset

Choices:

for now, does not implement

  • horizontal overflow dropdown feature
  • sub tabs feature
  • nav element feature
  • separate content (trigger) feature
  • child tab-panel mounting features
  • dynamic closable tabs feature
  • loading a tab via an external toggle

Testing Instructions

  • Load up the deploy preview for this branch
  • Load up PatternFly v4 Tabs
  • Compare and contrast
  • Read over the docs and see if anything's missing or no longer relevant

Notes to reviewers

This PR is BREAKING. It aligns pfe-tabs with pfv4.

zeroedin avatar Aug 19 '22 15:08 zeroedin

🦋 Changeset detected

Latest commit: 2b60b89633c9769453f5dba9e1114e29349c7d53

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

This PR includes changesets to release 2 packages
Name Type
@patternfly/pfe-core Minor
@patternfly/pfe-tabs Major

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 Aug 19 '22 15:08 changeset-bot[bot]

Deploy Preview for patternfly-elements ready!

Name Link
Latest commit 739b14dc84ec766d801230c1c383031d73ce53a9
Deploy Preview https://deploy-preview-2099--patternfly-elements.netlify.app/

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

github-actions[bot] avatar Aug 19 '22 15:08 github-actions[bot]

Should support icons in tabs per https://github.com/patternfly/patternfly-elements/discussions/2127#discussion-4335469

bennypowers avatar Aug 26 '22 10:08 bennypowers

Demo is broken until #2137 is resolved

zeroedin avatar Sep 02 '22 18:09 zeroedin

Warning this was forced pushed

zeroedin avatar Oct 07 '22 13:10 zeroedin

@zeroedin I noticed some weirdness with the overflow tabs, here is a recording.

https://drive.google.com/file/d/1DpAGcXg8-Q32Ow3jKzmZ4fJqzZruzwyL/view?usp=sharing

Everything else looks good.

coreyvickery avatar Oct 11 '22 16:10 coreyvickery

@zeroedin I noticed some weirdness with the overflow tabs, here is a recording.

Nice find @coreyvickery, turns out just a wrong CSS property mistake during a find & replacement was the culprit. Fix pushed.

zeroedin avatar Oct 11 '22 18:10 zeroedin

Well wanna go back and update the tests after switch lands, tho

Alrighty then, I will also have to rework what I just committed, assuming switch has element internals stuff in it. I thought I was missing something as I was struggling to get tests/code to work properly. All good, can revert it later.

zeroedin avatar Oct 19 '22 21:10 zeroedin

i can't shake the suspicion that we're bumping up on polyfill bugs

https://discord.com/channels/944894512717770762/945061812351692830/1037871477778829403

bennypowers avatar Nov 03 '22 23:11 bennypowers

I think this works fine in chrome if we just set the relevant properties on #internals

https://codepen.io/bennyp/pen/BaVKQja?editors=1011

if it's not working in polyfill browsers, we should fix the polyfill, I'm certain our PR will be reviewed in short order.

Likewise if FF's impl is broken we should file a bug

Screenshot from 2022-11-04 11-17-54

bennypowers avatar Nov 04 '22 09:11 bennypowers