Skip to content

feat: adding TagDiscovery panel feature (#494)#502

Open
EmanuelGF wants to merge 3 commits intolinkdotnet:masterfrom
EmanuelGF:feat-#494
Open

feat: adding TagDiscovery panel feature (#494)#502
EmanuelGF wants to merge 3 commits intolinkdotnet:masterfrom
EmanuelGF:feat-#494

Conversation

@EmanuelGF
Copy link
Contributor

Reopening the PR after the accidental merge and reset of master as discussed in #500.

@EmanuelGF
Copy link
Contributor Author

Reopened the PR after the accidental merge/reset.
Original discussion and review comments are in #500

@linkdotnet
Copy link
Owner

Taking over from the accidentially closed PR:

  • The build is currently red
  • We have to add a migration for the new property in the appsettings.json

@EmanuelGF
Copy link
Contributor Author

Thanks for the heads up @linkdotnet ! I noticed there are some issues with the tests related to the new dependencies I introduced, so I’ll work on fixing those. I can also address the remaining review suggestions from the previous PR.

@EmanuelGF
Copy link
Contributor Author

Addressed the remaining review suggestions given by @digitaldirk and fixed the failing tests related to the new dependencies. The Tag Discovery panel was also moved outside the navbar to prevent it from closing on resize. Can now be closed using escape key and display message when emply.

Have a great weekend!!

@linkdotnet
Copy link
Owner

Awesome - looking already good.

As we have a breaking change in the appsettings.json, we need a migration here: https://github.com/linkdotnet/Blog/tree/master/tools/LinkDotNet.Blog.UpgradeAssistant

Basically adding an entry in the ctor here: https://github.com/linkdotnet/Blog/blob/master/tools/LinkDotNet.Blog.UpgradeAssistant/MigrationManager.cs#L17

And adding the logic next to here: https://github.com/linkdotnet/Blog/blob/master/tools/LinkDotNet.Blog.UpgradeAssistant/Migrations/Migration_11_To_12.cs

Copy link
Contributor

@digitaldirk digitaldirk left a comment

Choose a reason for hiding this comment

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

Awesome work, the issues and suggestions I had are fixed :)

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.

3 participants