Add generic alert #395
Add generic alert #395
Conversation
jesusreal
requested review from
antiheld,
bszwarc,
dariadomagala-sap,
hardl,
JohannesDoberer,
kazydek,
klaudiagrz,
kwiatekus,
marynaKhromova,
maxmarkus,
parostatkiem-zz and
pekura
as code owners
Co-Authored-By: jesusreal <8238188+jesusreal@users.noreply.github.com>
Co-Authored-By: jesusreal <8238188+jesusreal@users.noreply.github.com>
core/src/Alert.html
Outdated
| }) | ||
| }); | ||
| processedData.links.forEach(l => { | ||
| this.addCLickListener(l.url, l.elemId); |
There was a problem hiding this comment.
here as well :)
| this.addCLickListener(l.url, l.elemId); | |
| this.addClickListener(l.url, l.elemId); |
There was a problem hiding this comment.
Already pushed that one :)
core/src/Alert.html
Outdated
| oncreate() { | ||
| const defaultSettings = { | ||
| text: | ||
| 'I am a Luigi alert with extremely relevant information for the user', |
There was a problem hiding this comment.
Should we display an alert with the default value? I don't see any values from showing this kind of message. Maybe we shouldn't show the alert at all in this situation?
There was a problem hiding this comment.
Great point. I will display a message in the browser console if an alert has no text and will not display the alert in the UI. Additionally, I will remove the default values for alerts, since this makes no sense:
const defaultSettings = {
type: 'info',
links: undefined
};
| class="fd-form__control" | ||
| data-cy="luigi-alert-text" | ||
| /> | ||
| With text (if alert has not text it is not displayed) |
There was a problem hiding this comment.
Tiny thing but: has no text 😉
There was a problem hiding this comment.
both updates are done
client/src/luigi-client.js
Outdated
| * @param {('info'|'success'|'warning'|'error')} settings.type sets the type of the alert | ||
| * @param {Object} settings.links provides links data | ||
| * @param {Object} settings.links.LINK_KEY object containing the data for a concrete link. | ||
| * Read above on how to use the key to for the link to be properly rendered in the alert message. |
There was a problem hiding this comment.
| * Read above on how to use the key to for the link to be properly rendered in the alert message. | |
| * To properly render the link in the alert message refer to the description of the **settings.links.LINK.KEY** parameter. |
There was a problem hiding this comment.
Almost, it is settings.text instead of settings.links.LINK.KEY. I will commit it separately.
Co-Authored-By: jesusreal <8238188+jesusreal@users.noreply.github.com>
Co-Authored-By: jesusreal <8238188+jesusreal@users.noreply.github.com>
Co-Authored-By: jesusreal <8238188+jesusreal@users.noreply.github.com>
Co-Authored-By: jesusreal <8238188+jesusreal@users.noreply.github.com>
Co-Authored-By: jesusreal <8238188+jesusreal@users.noreply.github.com>
Description
Changes proposed in this pull request:
Related issue(s)