4 features to review in this PR #213
Conversation
There was a problem hiding this comment.
- Remove toggle hover effect #148 : I will approve for now but I'd like to hear what @JohnPixle thinks about the toggle style
- Allow nesting code snippet shortcodes within code snippet shortcodes #198 : tested, works well
- https://github.com/codesnippetspro/pro/issues/19 : tested, works well
- Disable scroll to top after save #208 : tested, nice improvement 👍
|
@imantsk Thanks for pinging me here. One silly question, how can I test the file? 🙈 I tried to download the zip at the branch, but it did not work when I installed it. From what I imagine, in worst case we could just remove the hover style of the toggle altogether, as Atif suggested. It's a very common UI element and I don't think it's critical to provide a hint for the fact that users can interact with it. In any case, let me know how I can test it, happy to take a look! |
|
Some of these issues are eclipsed by the changes I've made when integrating the new UI, but I don't see anything wrong with making these changes for the current interface in the meantime. I'm happy to merge this and include it in the new patch if we're happy with testing. |
|
Thanks for helping me test this one. Regarding the toggle, to be honest I am not a big fan of the shadow on hover. As @lightbulbman already mentioned:
I agree with that. I believe our best bet is to simply remove the hover animation AND the background shadow. I don't think there is a need to imply interactivity here. So, to sum-up:
Let me know your thoughts! |
…/codesnippetspro/code-snippets into 19-add-snippet-name-to-shortcode
@sheabunge @imantsk - Please can you review the 4 changes made in this PR:
Manage Snippets Page, effect of hover on toggle (linked to issue 148) - user suggested that the movement of the toggle on hover is not a good UI experience and I agree, to enhance or give indication of interactivity I removed the movement on hover and added a background shadow. TBH most users will know what a toggle switch will do so we can even remove this altogether if feel there is no need.
Nesting shortcodes (linked to issue 198 - further description there) - by adding shortcodes [code-snippet "shortcodes"] without the "" in the code html snippet shortcode now allows the html snippet itself to contain another shortcode that will be rendered therefore allowing nested shortcodes. One limitation I just realised and haven't tested is that this may only allow one nested shortcode and not multiple, this could be an area of improvement for later on.
Shortcode name attribute (Issue 19) - all html snippets where a shortcode is produced now include a name attribute which is the snippet name/ title capped to 3 words with trailing dots if longer to not make the shortcode so long...e.g. new format [code-snippets id=5 name='Test Snippet']
Sticky admin notice (linked to issue 208 - principle based on user brandonjp suggestion) - When saving a long snippet or any snippet the default behaviour is a scroll to top which shows the save admin notice, this change now removes this behaviour and instead makes the admin notice sticky so that it appears towards the top of the browser window so that if a user is editing a long snippet then they are not scrolled to top of page, and removes the notice automatically after 5 seconds.