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

ConfigurationEditorOverlay: Make use of Umbraco directives and change <a> to <button> #44

Merged

Conversation

BatJan
Copy link
Contributor

@BatJan BatJan commented Oct 11, 2020

Description

I have changed <a> to <button> and replaced <i> with <umb-icon> directive and I have replaced the search filter markup with the <umb-search-filter> directive instead. Finally I have deleted the styling rule for the Umbraco icon setting it's height limit to 30pixels even though the .large class says to make the icons 32x32 pixels meaning the Umbraco icon would look like it was cut off in the bottom - So removed that selector from the file.

Types of changes

  • Documentation change
  • 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

  • My code follows the coding style of this project.
  • My changes generate no new warnings.
  • My change requires a change to the documentation.
  • I have updated the corresponding documentation.
  • I have read the CONTRIBUTING and CODE_OF_CONDUCT documents.

@BatJan BatJan requested a review from leekelleher as a code owner October 11, 2020 21:00
@leekelleher
Copy link
Owner

Thank you @BatJan! 🎉

Do you know which Umbraco version the <umb-icon> directive was introduced?

Currently Contentment builds against v8.6.1 (as that was the latest version at the time of my initial release). I'm considering bumping my Umbraco version dependency, but just need to figure out my own version numbering, (as with SemVer that'd be a v2.0). Nothing for you to worry about though, it's more about logistics for me. 😃

With the Umbraco icon, with v8.8.0, there's probably a better way to achieve this with an SVG icon. I'll keep that in mind. 👍

@leekelleher leekelleher added the hacktoberfest-accepted Pull request accepted as Hacktoberfest submission label Oct 12, 2020
@leekelleher
Copy link
Owner

...and I wasn't aware of the <umb-search-filter> directive either! (re: your PR #8179) 😀 🙌

@BatJan
Copy link
Contributor Author

BatJan commented Oct 12, 2020

You're most welcome Lee 😊 Oh yes, good point. I did not think about the versioning at all 😅 is available with the 8.8 release. I can't remember when was introduced actually - It might have been 8.6 or 8.7 😬

Do you want me to roll some of it back or will you bump up the release number? 🙂 Just let me know. I have another draft PR that I hope to finish during the week as well. I actually only need to write about the changes made so not much that needs to be done but with a baby in the house time and memory is a thing 😂

@leekelleher
Copy link
Owner

Carry on as you are @BatJan, all good.
I'll figure out the versioning.

@BatJan BatJan mentioned this pull request Oct 16, 2020
9 tasks
@leekelleher leekelleher mentioned this pull request Oct 25, 2020
leekelleher pushed a commit that referenced this pull request Dec 1, 2020
Changes taken from @BatJan's pull request.
Refactor view to use umbraco directives and change a to button
#44

Once Contentment's dependency on Umbraco has been bumped
up beyond v8.8.0, then I'll merge in the rest of the pull request.
@leekelleher
Copy link
Owner

@BatJan Just to let you know that I've finally got around to adding some of your accessibility amends to the current version, in commit: 0f22330. 🚀

Once I figure out when I can bump the Umbraco dependency up beyond v8.8.0 (for the other new directives), I'll merge the rest in. I'm hoping to ramp up my releases soon, (the gap between v1.0 and v1.1 has felt too long for me).

@leekelleher leekelleher changed the base branch from develop to dev/v2.0 December 22, 2020 15:42
Copy link
Owner

@leekelleher leekelleher left a comment

Choose a reason for hiding this comment

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

All good, thank you @BatJan! 🌟

@leekelleher
Copy link
Owner

I've decided that Contentment v2.x will build against Umbraco v8.10.0, (once it's released).
So I can now merge this in, thanks again!

@leekelleher leekelleher merged commit efa4ff1 into leekelleher:dev/v2.0 Dec 22, 2020
@leekelleher leekelleher added this to the 2.0.0 milestone Dec 22, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest-accepted Pull request accepted as Hacktoberfest submission
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants