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.

Sign up for GitHub

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

Jump to bottom

chore(Page) remove deprecated props #8185

Closed
wants to merge 22 commits into from
Closed

chore(Page) remove deprecated props #8185

wants to merge 22 commits into from

Conversation

Dominik-Petrik
Copy link
Contributor

What: Closes #8091

@patternfly-build
Copy link
Contributor

patternfly-build commented Oct 7, 2022 •
edited
Loading

Preview: https://patternfly-react-pr-8185.surge.sh

A11y report: https://patternfly-react-pr-8185-a11y.surge.sh

@Dominik-Petrik Dominik-Petrik marked this pull request as draft October 7, 2022 11:48
@Dominik-Petrik Dominik-Petrik marked this pull request as ready for review October 7, 2022 13:01
@Dominik-Petrik Dominik-Petrik linked an issue Oct 7, 2022 that may be closed by this pull request
[Page] - [remove deprecated props] #8091
Closed
@tlabaj tlabaj added breaking change 💥 this change requires a major release and has API changes. PF5 labels Oct 7, 2022
tlabaj
tlabaj approved these changes Oct 7, 2022
View reviewed changes
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

LGTM

thatblindgeye
thatblindgeye reviewed Oct 10, 2022
View reviewed changes
Copy link
Contributor

@thatblindgeye thatblindgeye left a comment

Choose a reason for hiding this comment

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

This is looking good! Had a couple of questions which @tlabaj may be able to weigh in on:

  1. The snapshots look to have been updated where the pf-m-sticky-[top/bottom] class is removed, and a sticky attribute is instead added. Should the pf-m-sticky class not still be applied in the snapshots?
  2. In the PageHeader component, the isManagedSidebar prop description mentions "This prop is no longer managed through PageHeader but in the Page component", and the prop is also given the alias of deprecatedIsManagedSidebar on line 46. Would we want to remove this prop as well or add a proper @deprecated in the description?

@tlabaj
Copy link
Contributor

tlabaj commented Oct 10, 2022

This is looking good! Had a couple of questions which @tlabaj may be able to weigh in on:

  1. The snapshots look to have been updated where the pf-m-sticky-[top/bottom] class is removed, and a sticky attribute is instead added. Should the pf-m-sticky class not still be applied in the snapshots?
  2. In the PageHeader component, the isManagedSidebar prop description mentions "This prop is no longer managed through PageHeader but in the Page component", and the prop is also given the alias of deprecatedIsManagedSidebar on line 46. Would we want to remove this prop as well or add a proper @deprecated in the description?

@thatblindgeye is correct.

  1. It should still be pf-m-sticky-[top/bottom] for the modifier.
  2. We should removed the isManagedSidebar prop and the associated generated console warning.

tlabaj
tlabaj requested changes Oct 10, 2022
View reviewed changes
Copy link
Contributor

@tlabaj tlabaj left a comment

Choose a reason for hiding this comment

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

Please address @thatblindgeye comments

@Dominik-Petrik Dominik-Petrik force-pushed the PageDeprecatedProps branch 2 times, most recently from 2d02681 to 33ee401 Compare October 13, 2022 10:57
@Dominik-Petrik Dominik-Petrik marked this pull request as draft October 13, 2022 11:04
thatblindgeye and others added 16 commits October 13, 2022 13:06
@thatblindgeye @Dominik-Petrik
fix(Menu): fixed height issue with drilldown examples (patternfly#8033)
1b6f860
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
276c05f 
 - @patternfly/react-catalog-view-extension@4.92.27
 - @patternfly/react-code-editor@4.82.27
 - @patternfly/react-console@4.92.27
 - @patternfly/react-core@4.250.2
 - @patternfly/react-docs@5.102.32
 - @patternfly/react-inline-edit-extension@4.86.28
 - demo-app-ts@4.202.8
 - @patternfly/react-log-viewer@4.87.22
 - @patternfly/react-table@4.111.5
 - @patternfly/react-topology@4.88.27
 - @patternfly/react-virtualized-extension@4.88.27
@tlabaj @Dominik-Petrik
chore(docs): Release notes 2022.13 (patternfly#8168)
7cc3d53 
* chore(docs): Release notes 2022.13

* release note updates

* add screnshots and versions

* update note

* updates from comments

Co-authored-by: Titani <tlabaj@redaht.com>
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
d6b9cb6 
 - @patternfly/react-docs@5.102.33
@tlabaj @Dominik-Petrik
chore(docs): Updated release notes (patternfly#8202)
8fb1fe6 
Co-authored-by: Titani <tlabaj@redaht.com>
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
7189cbd 
 - @patternfly/react-docs@5.102.34
@andyyvo @Dominik-Petrik
chore(Sidebar): converted examples to typescript (patternfly#8062)
37c3d1b
@samuelatefah @Dominik-Petrik
chore(Banner): update tests to new react testing library standards (p…
331ddc8 
…atternfly#8160)

* chore(Banner): update tests to new react testing library standards

* add additional test to test screenReaderText

* replace toHaveTextContent with toBeInTheDocument matcher

* add test to check for pf-u-screen-reader class
@gitdallas @Dominik-Petrik
specify est timezone for jest to run with (patternfly#8151)
ad9456d
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
fe3eeca 
 - @patternfly/react-catalog-view-extension@4.92.28
 - @patternfly/react-code-editor@4.82.28
 - @patternfly/react-console@4.92.28
 - @patternfly/react-core@4.250.3
 - @patternfly/react-docs@5.102.35
 - @patternfly/react-inline-edit-extension@4.86.29
 - demo-app-ts@4.202.9
 - @patternfly/react-log-viewer@4.87.23
 - @patternfly/react-table@4.111.6
 - @patternfly/react-topology@4.88.28
 - @patternfly/react-virtualized-extension@4.88.28
@gitdallas @Dominik-Petrik
feature(Select): flag to put create option at top of typeahead (patte…
48efe27 
…rnfly#8165)
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
bc37eff 
 - @patternfly/react-catalog-view-extension@4.92.29
 - @patternfly/react-code-editor@4.82.29
 - @patternfly/react-console@4.92.29
 - @patternfly/react-core@4.250.4
 - @patternfly/react-docs@5.102.36
 - @patternfly/react-inline-edit-extension@4.86.30
 - demo-app-ts@4.202.10
 - @patternfly/react-log-viewer@4.87.24
 - @patternfly/react-table@4.111.7
 - @patternfly/react-topology@4.88.29
 - @patternfly/react-virtualized-extension@4.88.29
@tlabaj @Dominik-Petrik
fix(Dropdown next): Add support for forward ref and updated docs. (pa…
7466616 
…tternfly#8142)

* fix(Dropdown next): Add support for forward ref and updated docs.

* hide inner ref prop

* hide innerRef

Co-authored-by: Titani <tlabaj@redaht.com>
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
a8d9c63 
 - @patternfly/react-catalog-view-extension@4.92.30
 - @patternfly/react-code-editor@4.82.30
 - @patternfly/react-console@4.92.30
 - @patternfly/react-core@4.250.5
 - @patternfly/react-docs@5.102.37
 - @patternfly/react-inline-edit-extension@4.86.31
 - demo-app-ts@4.202.11
 - @patternfly/react-log-viewer@4.87.25
 - @patternfly/react-table@4.111.8
 - @patternfly/react-topology@4.88.30
 - @patternfly/react-virtualized-extension@4.88.30
@drewamunat2 @Dominik-Petrik
chore(Title): update tests to new RTL standards (patternfly#8156)
a5d1410 
* chore(Title): update tests to new RTL standards

* chore(Title): update tests to new RTL standards

* chore(Title): update tests to new RTL standards

Co-authored-by: Drew Amunategui II <drewamunateguiii@drews-mbp.acasmart.jh.edu>
Co-authored-by: Drew Amunategui II <drewamunateguiii@Drews-MacBook-Pro.local>
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
b5aad44 
 - @patternfly/react-catalog-view-extension@4.92.31
 - @patternfly/react-code-editor@4.82.31
 - @patternfly/react-console@4.92.31
 - @patternfly/react-core@4.250.6
 - @patternfly/react-docs@5.102.38
 - @patternfly/react-inline-edit-extension@4.86.32
 - demo-app-ts@4.202.12
 - @patternfly/react-log-viewer@4.87.26
 - @patternfly/react-table@4.111.9
 - @patternfly/react-topology@4.88.31
 - @patternfly/react-virtualized-extension@4.88.31
tlabaj and others added 6 commits October 13, 2022 13:06
@tlabaj @Dominik-Petrik
docs(Empty state): Added EmptyStatePrimary to documentation (patternf…
c53acd3 
…ly#8161)

* docs(Empty state): Added EmptyStatePrimary to documentation

* fix typo

Co-authored-by: Titani <tlabaj@redaht.com>
@patternfly-build @Dominik-Petrik
chore(release): releasing packages [ci skip]
47eef9d 
 - @patternfly/react-catalog-view-extension@4.92.32
 - @patternfly/react-code-editor@4.82.32
 - @patternfly/react-console@4.92.32
 - @patternfly/react-core@4.250.7
 - @patternfly/react-docs@5.102.39
 - @patternfly/react-inline-edit-extension@4.86.33
 - demo-app-ts@4.202.13
 - @patternfly/react-log-viewer@4.87.27
 - @patternfly/react-table@4.111.10
 - @patternfly/react-topology@4.88.32
 - @patternfly/react-virtualized-extension@4.88.32
@Dominik-Petrik
chore(Page) remove deprecated props
d222cfb
@Dominik-Petrik
update tests
25a8b8d
@Dominik-Petrik
update docs and integration tests
8c1cf35
@Dominik-Petrik
fix snapshots, remove isManagedSidebar prop
5860111
@Dominik-Petrik Dominik-Petrik deleted the PageDeprecatedProps branch October 13, 2022 14:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@thatblindgeye thatblindgeye thatblindgeye left review comments

@tlabaj tlabaj tlabaj requested changes

Assignees
No one assigned
Labels
breaking change 💥 this change requires a major release and has API changes. PF5
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[Page] - [remove deprecated props]
8 participants
@Dominik-Petrik @patternfly-build @tlabaj @thatblindgeye @andyyvo @samuelatefah @gitdallas @drewamunat2