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.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate some component attributes #1722

Merged
merged 5 commits into from
Jan 19, 2022

Conversation

jovyntls
Copy link
Contributor

@jovyntls jovyntls commented Jan 17, 2022

What is the purpose of this pull request?

  • Documentation update
  • Bug fix
  • Feature addition or enhancement
  • Code maintenance
  • Others, please explain:

Fixes #1504

Overview of changes:

  • Removes the implementation of the following deprecated attributes:
Component Current attribute name Deprecated attribute name
Dropdown header text
Box header heading
Popover header title
Modal header title
  • Using these attributes will no longer raise a warning.
  • These attributes are also removed from the documentation.

Anything you'd like to highlight / discuss:
The _warnDeprecatedAttributes function was removed as well.

Also, I notice that the master branch doesn't seem to be passing all tests on npm run test (not as a result of this PR), but this can be fixed with npm run updatetest.
Should commit the files generated by npm run updatetest here as well? The non-updated tests are not a result of this PR so I'm not sure if it should be in the scope of this PR. 馃槄

Proposed commit message: (wrap lines at 72 characters)

Deprecate attributes for Dropdown, Box, Popover, Modal

Checklist: 鈽戯笍

  • Updated the documentation for feature additions and enhancements
  • Added tests for bug fixes or features
  • Linked all related issues
  • No unrelated changes

@jovyntls jovyntls marked this pull request as ready for review January 17, 2022 14:00
@tlylt
Copy link
Contributor

tlylt commented Jan 18, 2022

Also, I notice that the master branch doesn't seem to be passing all tests on npm run test (not as a result of this PR), but this can be fixed with npm run updatetest

I just ran the tests again locally on master and was able to pass all.
Curious have you tried

  • npm run setup and then npm run test to ensure that all dependencies are up to date? Else, perhaps a screenshot of the failing test would be great:)
  • ensured that the master branch of your fork is in sync
    image

@jovyntls
Copy link
Contributor Author

Thanks @tlylt , that works!

Have updated the issue description :)

Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Nice work @jovyntls! A couple of suggestions. Otherwise the rest looks good.

Thanks @tlylt for helping out 馃憤

packages/core/test/unit/html/NodeProcessor.data.js Outdated Show resolved Hide resolved
packages/core/test/unit/html/NodeProcessor.data.js Outdated Show resolved Hide resolved
docs/userGuide/syntax/boxes.mbdf Outdated Show resolved Hide resolved
Copy link
Contributor

@jonahtanjz jonahtanjz left a comment

Choose a reason for hiding this comment

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

Thanks for making the changes @jovyntls :)

LGTM 馃憤

@jonahtanjz jonahtanjz added this to the 4.0 milestone Jan 18, 2022
@jonahtanjz jonahtanjz merged commit c402024 into MarkBind:master Jan 19, 2022
@jonahtanjz jonahtanjz modified the milestones: 4.0, 3.1.1 Jan 22, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Fully deprecate some component attributes
3 participants