Skip to content

refactor(ui5-popup): Move more common functionality to Popup.js #1853

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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 9 commits into from
Jun 23, 2020

Conversation

vladitasev
Copy link
Contributor

@vladitasev vladitasev commented Jun 22, 2020

Currently Dialog.js and Popover.js have a lot in common in the open/openBy and close methods.

The goal of this change is to streamline them, and at the same time make Popup.js much more self-sufficient, so that third parties can create custom popups without understanding our low-level APIs such as popup registries, body scroll blocking, etc...

Popup.js:

  • is fully complete and functional on its own - it is enough to inherit it and implement isModal
  • has no notion of headerText, header and footer, only a default slot
  • the hooks in the template are now called beforeContent and afterContent
  • has almost no CSS, only the bare-bone styles to make it work

Dialog.js:

  • is now very thin as the whole open/close logic has moved to Popup.js
  • Implements the 2 hooks in the .hbs

Popover.js:

  • only wraps open in openBy and has no close since the code is now in Popup.js
  • is refactored a bit with several hooks to make it work with the base popup logic
  • also implements the 2 hooks in the .hbs

closes: #1837
closes: #1836

@@ -18,36 +19,14 @@ const metadata = {
slots: /** @lends sap.ui.webcomponents.main.Popup.prototype */ {

/**
* Defines the content of the Web Component.
* Defines the content of the Popup.
Copy link
Member

Choose a reason for hiding this comment

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

Really minor, but this "Popup" text would appear in the Dialog API as well. If we consider the term Popup good for the Dialog it's ok, otherwise we should fix it.

@vladitasev vladitasev merged commit a5d3dd7 into master Jun 23, 2020
@vladitasev vladitasev deleted the custom-popup-2 branch June 23, 2020 08:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants