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

Header ID shouldn't be mandatory! #125

Closed
Windos opened this issue Oct 7, 2020 · 4 comments · Fixed by #127
Closed

Header ID shouldn't be mandatory! #125

Windos opened this issue Oct 7, 2020 · 4 comments · Fixed by #127
Assignees
@Windos
Labels
enhancement good first issue HACKTOBERFEST A good target for #Hacktoberfest PRs. Learn, contribute, earn a shirt! help wanted

Comments

@Windos
Copy link
Owner

Windos commented Oct 7, 2020

Summary of the new feature/enhancement

New-BTHeading requires an ID as this is required in the Windows APIs. It is how Windows knows to group multiple notifications under the same heading.

This feels a little cumbersome for most use cases, surely it could be dynamic?

Proposed technical implementation details (optional)

Perhaps if an ID isn't specified, the function should automatically re-use the title...
@Windos Windos added enhancement help wanted HACKTOBERFEST A good target for #Hacktoberfest PRs. Learn, contribute, earn a shirt! good first issue labels Oct 7, 2020
@Windos Windos self-assigned this Oct 7, 2020
@glennsarti
Copy link
Contributor

@Windos May I have a go at this?

@Windos
Copy link
Owner Author

Windos commented Oct 7, 2020

@glennsarti go for it!

glennsarti added a commit to glennsarti/BurntToast that referenced this issue Oct 8, 2020
@glennsarti
(WindosGH-125) Make Header ID optional
06f9119 
Previously the Id parameter to New-BTHeader was mandatory however as it can be
a generated unique string it doesn't have to be.  This commit modifies the
Id parameter so it is no longer mandatory and uses a GUID style string as the
autogenerated Id if none is passed.  GUIDs are used instead of, say, the title
because they are guaranteed to be unique whereas using the title or date/time
could cause notification Id collisions.

This commit also adds a test to ensure that the function can be called without
the -Id parameter and that it is actually generated, not left as null or an
empty string.
@glennsarti glennsarti mentioned this issue Oct 8, 2020
Merged
@steviecoaster
Copy link
Contributor

Dang it Glenn, beatin me too things 😂

Windos added a commit that referenced this issue Oct 21, 2020
@Windos
Merge pull request #127 from glennsarti/gh-125-optional-id
0f3e32e 
(GH-125) Make Header ID optional
@Windos
Copy link
Owner Author

Windos commented Oct 21, 2020

And Glenn nailed it 😄

@Windos Windos closed this as completed Oct 21, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@Windos Windos

Labels
enhancement good first issue HACKTOBERFEST A good target for #Hacktoberfest PRs. Learn, contribute, earn a shirt! help wanted
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

(GH-125) Make Header ID optional glennsarti/BurntToast
3 participants
@glennsarti @Windos @steviecoaster