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

ContentBlocksOverlay: Various changes #45

Merged
merged 2 commits into from
Dec 22, 2020
Merged

ContentBlocksOverlay: Various changes #45

merged 2 commits into from
Dec 22, 2020

Conversation

BatJan
Copy link
Contributor

@BatJan BatJan commented Oct 11, 2020 •
edited
Loading

Description

Ok so when I committed this someone needed a bottle and some comforting and here we are 5 days later with me trying to remember what I did 😅

Given the info mentioned in #44 I suspect this one might be a breaking change on Umbraco 8.6.1 🤔 So I marked that checkbox instead this time.

I converted a number of <a> to <button> a number of places. I also added the <umb-icon> directive where appropriate - Also i wrapped an icon that previously had a click event attached directly to be wrapped inside a <button> instead wapping the <umb-icon> directive. Oh yes and then I added a screen reader friendly text instead too 👍

Please let me know if I broke something and did not realise it 😅

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 marked this pull request as ready for review October 16, 2020 13:45
@BatJan BatJan requested a review from leekelleher as a code owner October 16, 2020 13:45
@leekelleher leekelleher added the hacktoberfest-accepted Pull request accepted as Hacktoberfest submission label Oct 25, 2020
@leekelleher leekelleher mentioned this pull request Oct 25, 2020
Closed
leekelleher pushed a commit that referenced this pull request Dec 1, 2020
@BatJan @leekelleher
ContentBlocks - accessibility amends
7c721ea 
Changes taken from @BatJan's pull request.
Convert a and i to button and umb-icon
#45

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: 7c721ea. 🚀

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

@BatJan I can merge this in now. I've decided that Contentment v2.x will build against Umbraco v8.10.0, (once it's released).
Thanks again for these contributions, they are very much appreciated!

@leekelleher leekelleher merged commit 4207e1d 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
Reviewers

@leekelleher leekelleher leekelleher left review comments

Assignees
No one assigned
Labels
hacktoberfest-accepted Pull request accepted as Hacktoberfest submission
Projects
None yet
Milestone
2.0.0
Development

Successfully merging this pull request may close these issues.
2 participants
@BatJan @leekelleher