-
Notifications
You must be signed in to change notification settings - Fork 32
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
Add contact block to news page and improve buttons #747
Conversation
Affected libs:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work! I like the refactor of the button logic! Thanks @f-necas
@@ -132,9 +132,6 @@ describe('DownloadsListComponent', () => { | |||
LINK_FIXTURES.geodataShpWithMimeType | |||
) | |||
expect(items[0].componentInstance.format).toEqual('shp') | |||
expect(items[0].componentInstance.color).toEqual( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this change necessary in this PR? I'm just wondering why this got removed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is because the color getter is deleted. So we can't call it anymore : https://github.com/geonetwork/geonetwork-ui/pull/747/files#diff-5cfed937e30ec743c98e5e024350945ee6d4d0b6e7e34dabac9f3fb580fa0ad4L27-L28
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that shouldn't be impacted here, unless I'm missing something; in general we should strive to avoid reducing the test coverage :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh yeah indeed, I don't know why my IDE displayed it in red. Sry @Angi-Kinas for the misunderstood !
<h2 | ||
class="text-2xl font-title text-primary leading-7 text-left" | ||
translate | ||
> | ||
datahub.news.contact.title | ||
</h2> | ||
<p translate>datahub.news.contact.desc1</p> | ||
<p translate>datahub.news.contact.desc2</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Note that you can here simply use a translation key that accepts HTML, see for instance https://github.com/geonetwork/geonetwork-ui/blob/main/translations/en.json#L112
instead of hardcoding a structure with a title and paragraphs
conf/default.toml
Outdated
@@ -34,6 +34,9 @@ proxy_path = "" | |||
# More information about the translation can be found in the docs (https://geonetwork.github.io/geonetwork-ui/main/docs/reference/i18n.html) | |||
# languages = ['en', 'fr', 'de'] | |||
|
|||
# Allows displaying a contact email on news page for missing data |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
# Allows displaying a contact email on news page for missing data | |
# Enables displaying a "contact block" wherever relevant in applications |
67995b7
to
a2973a5
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the rework of the button component. It reduces the logic and complexity :-) 👍
I'll approve the PR, but it should not be merged yet because of the new release of the 2.1.0 version.
Co-authored-by: Angi-Kinas <133115263+Angi-Kinas@users.noreply.github.com>
63e40b7
to
7e28879
Compare
Contact block & buttons
Contact
Add a contact block with a configurable mail in
default.toml
How-to
Add mail in
default.toml
Buttons improvement
As
gn-ui-button
isn't really necessary, this PR also propose to improve them with tailwind classes intailwind.base.css
.This is the first (without breaking changes) step to get rid of
gn-ui-button
.