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

feat(toast): property to specify an element as toast content instead … #436

Merged
merged 1 commit into from
Dec 14, 2023

Conversation

danice
Copy link

@danice danice commented Nov 21, 2023

Proposed changes

Alternative solution to "[Bug]: Toast html/unsafeHtml not work #394 ". This will allow to specify some element as toast content. A new property toastId will allow specify an element as tooltip content instead of text.

Screenshots (if appropriate) or codepen:

will appear as (this also shows part of corresponding update in docs)

imagen

Types of changes

  • Bug fix (non-breaking change which fixes an issue).
  • New feature (non-breaking change which adds functionality).
  • Breaking change (fix or feature that would cause existing functionality to change).

Checklist:

  • [ x I have read the CONTRIBUTING document.
  • My commit messages follows the conventional commit format
  • My change requires a change to the documentation, and updated it accordingly.
  • I have added tests to cover my changes.
  • All new and existing tests passed.

Copy link
Member

@wuda-io wuda-io left a comment

Choose a reason for hiding this comment

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

LGTM

@wuda-io
Copy link
Member

wuda-io commented Dec 6, 2023

Can you add also an additional PR in the docs Repo?
I still need to update the Readme Files how we handle the documentation changes in future.

Is there any best practice or do you have any ideas how we should handle this?
Thanks

@danice
Copy link
Author

danice commented Dec 6, 2023

Can you add also an additional PR in the docs Repo?

I did also the corresponding update in the docs, see here.

BTW as this uses M element from html I think this requires the other PR in docs to be merged first.

@danice
Copy link
Author

danice commented Dec 6, 2023

Is there any best practice or do you have any ideas how we should handle this?

The docs project is where I work, doing the changes in the library (the git submodule), and also in the docs itself if something is needed to test the changes or update the documentation.

So after the check-in of the changes is done in the library, also a check-in in the parent docs project should be done including the library commit with the changes and also the documentation changes.

What I would suggest is to have a branch in docs equivalent to the v2-dev branch in library. This branch will point to materializecss submodule v2-dev and is where changes in docs corresponding to library changes should start and be merged.

This will avoid the current situation where docs was pointing to 2.0.3-beta and when I merged the toast-content branch it looked as a lot of changes:

imagen

If I had started from a docs "v2-dev" branch pointing to "Merge pull request #435" and done the modifications from there the changes in the library will show only the change in one file, not as now that shows also all the differences between 2.0.3-beta and v2-dev:

imagen

So summarising: create a "v2-dev" branch in docs, do a new check-in with just materialize submodule pointing to its own v2-dev and do the changes in docs from there. When desired this branch could be merged to the release branch.

@wuda-io
Copy link
Member

wuda-io commented Dec 14, 2023

Oh yes that makes totally sense, i will try to update the branch confirguration.

@wuda-io wuda-io marked this pull request as ready for review December 14, 2023 22:39
@wuda-io wuda-io merged commit d053c91 into materializecss:v2-dev Dec 14, 2023
2 checks passed
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.

2 participants