search

noi-techpark / it.bz.opendatahub.databrowser

Explore and navigate through Open Data you need to build your next service.
https://databrowser.opendatahub.com
GNU Affero General Public License v3.0
8 stars 7 forks source link

Feat/target blank header links #510

Closed a-crea closed 5 months ago

a-crea commented 5 months ago

This PR add _blank target to external header links. It also includes the popover menu for login/register on desktop.

gappc commented 5 months ago

@a-crea the PR looks good to me, the only thing that we should discuss is the usage of the IconParser, e.g. here

The problem is related to issue #507, which shows that we have lots of warnings during compilation due to the mixed usage of IconParser (with dynamic icon imports) and static import of icon components. It seems that we need to stick to either the dynamic or static import. If you want, please tell your opinion in #507, thx

RudiThoeni commented 5 months ago

is this PR ready to merge?

MatteoBiasi commented 5 months ago

@gappc Since IconParser is present in the code for more than one year, if @sseppi agrees I suggest to open a separate issue for this and discuss it separately, so meanwhile we can close this one.

gappc commented 5 months ago

@MatteoBiasi yes, let's discuss this in a separate issue (quick note: the warnings showed up after the update of vite in 22542a37dc67b4abda9d51cd36833bf1bd749d22)

sseppi commented 5 months ago

@MatteoBiasi yes, let's discuss this in a separate issue (quick note: the warnings showed up after the update of vite in 22542a3)

@gappc can you please create a separate issue for the IconParser and assign it to me, so we can plan it in the next iterations?

gappc commented 5 months ago

@sseppi @MatteoBiasi @a-crea I've created issue #507, lets move the discussion there

sseppi commented 5 months ago

@sseppi @MatteoBiasi @a-crea I've created issue #507, lets move the discussion there

OK, super! Thank you!