Hybridize alert. #13
Hybridize alert. #13
Conversation
| @@ -133,21 +158,19 @@ | |||
| <div class="message-highlight"></div> | |||
| <slot></slot> | |||
| </div> | |||
| <button is="d2l-button" on-tap="_dispatchButtonPressedEvent" hidden$="[[!hasButton]]"> | |||
There was a problem hiding this comment.
Turns out we weren't relying on d2l-button for anything, so there wasn't any need to switch to d2l-button hybrid. In fact, doing so makes things difficult since it would require lots of style overrides.
| @@ -92,6 +111,7 @@ | |||
| padding: 1.15em 1.43em; | |||
| border-left: none; | |||
| font: inherit; | |||
| margin: 0; | |||
There was a problem hiding this comment.
This was needed just for Safari.
| @@ -1,66 +1,100 @@ | |||
| # d2l-alert | |||
| [](https://www.webcomponents.org/element/BrightspaceUI/alert) | |||
There was a problem hiding this comment.
Will still need to get this component registered.
| type: { | ||
| type: String, | ||
| value: 'call-to-action', | ||
| reflectToAttribute: true | ||
| }, | ||
| hasButton: { | ||
| _hasButton: { |
There was a problem hiding this comment.
Made this computed property private (ala "_" prefix), since it's not really part of the public API, and we don't really want it showing as such in the generated docs.
.editorconfig
Outdated
| @@ -8,6 +8,6 @@ insert_final_newline = true | |||
| indent_style = tab | |||
| indent_size = 4 | |||
|
|
|||
| [{*.json,*.yml}] | |||
| [{package.json,bower.json,.travis.yml,.eslintrc}] | |||
There was a problem hiding this comment.
Any reason for changing from the .json extension to no extension? I've been switching things to be the opposite lately as I think it's more standard.
There was a problem hiding this comment.
Haha - I was trying to follow new convention... since I think someone recently recommended I should omit the extension. Looks like text-input omits it too.
There was a problem hiding this comment.
Ha so I wouldn't trust text-input totally. Their docs seem to like having the extension.
README.md
Outdated
| A [Polymer](https://www.polymer-project.org/1.0/)-based web component for alerts. | ||
| [Polymer](https://www.polymer-project.org)-based web component for D2L alerts. | ||
|
|
||
|  |
There was a problem hiding this comment.
Thanks. I will correct. Copy & paste. ;)
|
I am also playing around with inline demo. I am gonna try and add it to see how it looks. |
| hasCloseButton: { | ||
| type: Boolean, | ||
| value: false | ||
| } | ||
| }, | ||
|
|
||
| /** | ||
| * Closes/hides the alert. | ||
| */ |
There was a problem hiding this comment.
I think ideally we'd want this to be a closed attribute, but that's a major breaking change so we can defer for now.
package.json
Outdated
| "eslint": "^3.14.0", | ||
| "eslint-config-brightspace": "^0.2.4", | ||
| "eslint-plugin-html": "^1.7.0", | ||
| "polymer-cli": "^0.16.0" | ||
| "polymer-cli": "^1.2.0" |
There was a problem hiding this comment.
You can likely go higher on the CLI version... I think it's up to 1.5 or higher now.
There was a problem hiding this comment.
Ok cool - I will update.
|
Got Inline Demo working. It's just a preview. I seemed to get a lot of errors initially - I think they do some processing on first request, and it takes some time for it. Had to keep refreshing the page, but once loaded it seemed to work fine. Anyway, assuming it loads, you can update the code, for example by changing the The Polymer folks seem to place these at the top of the readme files, but we could put it anywhere in the readme. |
No description provided.