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

Add contact block to news page and improve buttons #747

Merged
merged 5 commits into from
Jan 9, 2024

Conversation

f-necas
Copy link
Collaborator

@f-necas f-necas commented Jan 2, 2024

Contact block & buttons

Contact

Add a contact block with a configurable mail in default.toml
image

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 in tailwind.base.css.

This is the first (without breaking changes) step to get rid of gn-ui-button.

Copy link
Contributor

github-actions bot commented Jan 2, 2024

Affected libs: ui-inputs, feature-dataviz, feature-record, feature-router, feature-editor, feature-search, feature-map, ui-elements, feature-catalog, ui-catalog, ui-search, util-app-config,
Affected apps: datahub, metadata-converter, metadata-editor, demo, webcomponents, search, map-viewer, datafeeder,

  • 🚀 Build and deploy storybook and demo on GitHub Pages
  • 📦 Build and push affected docker images

@coveralls
Copy link

coveralls commented Jan 2, 2024

Coverage Status

coverage: 82.601% (-0.04%) from 82.639%
when pulling 7e28879 on DH/add-contact-to-news-page
into ed6fbd6 on main.

Copy link
Collaborator

@Angi-Kinas Angi-Kinas left a 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(
Copy link
Collaborator

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.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Copy link
Collaborator

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 :)

Copy link
Collaborator Author

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 !

@f-necas f-necas requested a review from Angi-Kinas January 3, 2024 09:04
Comment on lines 27 to 34
<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>
Copy link
Collaborator

@jahow jahow Jan 3, 2024

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

@@ -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
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
# Allows displaying a contact email on news page for missing data
# Enables displaying a "contact block" wherever relevant in applications

@f-necas f-necas requested a review from jahow January 3, 2024 14:57
@f-necas f-necas force-pushed the DH/add-contact-to-news-page branch 2 times, most recently from 67995b7 to a2973a5 Compare January 3, 2024 15:10
Copy link
Collaborator

@Angi-Kinas Angi-Kinas left a 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.

conf/default.toml Outdated Show resolved Hide resolved
@f-necas f-necas force-pushed the DH/add-contact-to-news-page branch from 63e40b7 to 7e28879 Compare January 9, 2024 16:31
@f-necas f-necas merged commit 9cf95ec into main Jan 9, 2024
7 checks passed
@f-necas f-necas deleted the DH/add-contact-to-news-page branch January 9, 2024 16:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants