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

Already on GitHub? Sign in to your account

Feature/toast component new #112

Closed

Conversation

marinakostic
Copy link

feat(toast): add new toast component

dgonzalezr and others added 30 commits January 19, 2023 11:40
Add `libs/bee-q` path to the output array of the build to be taking into account by Nx cache
Remove unnecessary dependsOn.
This method offers a convenient API and type safety to set attributes. Is primarily used to set multiple attributes at once.
…Endava#91)

Docs updated for Angular output target (@bee-q/angular package)
Co-authored-by: Dabiel González Ramos <[email protected]>
Icon SVGs used under the hood on the icon components come from the Phosphor icon library, which now has moved the SVG assets to a core package: https://github.com/phosphor-icons/core.

Details: https://github.com/phosphor-icons/web/releases/tag/v2.0.0
Bumps [rimraf](https://github.com/isaacs/rimraf) from 4.1.2 to 4.3.0.
- [Release notes](https://github.com/isaacs/rimraf/releases)
- [Changelog](https://github.com/isaacs/rimraf/blob/main/CHANGELOG.md)
- [Commits](isaacs/rimraf@v4.1.2...v4.3.0)

---
updated-dependencies:
- dependency-name: rimraf
  dependency-type: direct:development
  update-type: version-update:semver-minor
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Bumps [@types/node](https://github.com/DefinitelyTyped/DefinitelyTyped/tree/HEAD/types/node) from 18.14.1 to 18.14.6.
- [Release notes](https://github.com/DefinitelyTyped/DefinitelyTyped/releases)
- [Commits](https://github.com/DefinitelyTyped/DefinitelyTyped/commits/HEAD/types/node)

---
updated-dependencies:
- dependency-name: "@types/node"
  dependency-type: direct:development
  update-type: version-update:semver-patch
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
// Requires JSDocs for public API documentation.
// ===============================================

/** Trigers function to show toast */
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/** Trigers function to show toast */
/** Triggers function to show toast */

// render() function
// Always the last one in the class.
// ===================================
private getColorAndIcon = () => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please move this one between

 // Local methods
  // Internal business logic.
  // These methods cannot be called from the host element.
  // =======================================================

  // render() function
  // Always the last one in the class.
  // ===================================

private getColorAndIcon = () => {
const type = this.type;
const textColor = this.textColor;
const defaultColors = {
Copy link
Collaborator

Choose a reason for hiding this comment

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

It can be split into two functions
private getColorAndIcon = () => {

private get iconColor(){}

private get icon(){}

this way you can just call it directly as property even though is computed this.iconColor


/** Trigers function to show toast */
@Method()
async showToast() {
Copy link
Collaborator

Choose a reason for hiding this comment

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

We should provide a method to hide toast if the user doesn't want it to autoclose.


// Prop lifecycle events
// =======================
@Watch('type')
Copy link
Collaborator

@endv-bogdanb endv-bogdanb Mar 29, 2023

Choose a reason for hiding this comment

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

make sure the timer is always >=0

if(this.autoCloseTime<0){
      this.autoCloseTime = Math.max(0, this. autoCloseTime);
}
Suggested change
@Watch('type')
@Watch('type')
@Watch('autoCloseTime')

/** Should show icon of Toast */
@Prop({ reflect: true }) showIcon = false;
/** Should hide Toast after period of time */
@Prop({ reflect: true }) autoCloseTime: number;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I would make this property accept a null as well and default it to 5000ms if null is provided it will stay open until hide method is called

@Method()
async showToast() {
this.shouldShowToast = true;
const timeout = !this.autoCloseTime ? 5000 : this.autoCloseTime;
Copy link
Collaborator

Choose a reason for hiding this comment

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

We need the timeout as property. Maybe check the usage of debounce util, I think we can leverage that to auto close the toaster. Also it provides a cancel that can be called if the user decides to close it faster.

@endv-bogdanb
Copy link
Collaborator

I would add bqClose event, that's triggered when the toaster is closed.

@dgonzalezr dgonzalezr added feature 🛠 New feature or request don't merge 🚫 Pull request is on hold, please do not merge labels May 3, 2023
@endv-bogdanb
Copy link
Collaborator

Close in favour of #301

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
don't merge 🚫 Pull request is on hold, please do not merge feature 🛠 New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants