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

WIP: Isolate Drawer menu from the rest of the UI #3773

Draft
wants to merge 22 commits into
base: 9.0
Choose a base branch
from

Conversation

grebaldi
Copy link
Contributor

@grebaldi grebaldi commented Apr 30, 2024

refs: #3119
previous: #3682
depends on: #3792, #3804

WIP.

@github-actions github-actions bot added the 9.0 label Apr 30, 2024
@grebaldi grebaldi force-pushed the feature/multi-app/02-loosen-the-menu branch 2 times, most recently from 5666f1d to acbdd0a Compare May 30, 2024 11:05
@grebaldi grebaldi force-pushed the feature/multi-app/02-loosen-the-menu branch from 4c78c28 to ebaf71e Compare July 5, 2024 13:00
markusguenther pushed a commit that referenced this pull request Jan 14, 2025
Prelude to: #3773
refs: #3119

This PR reforms the UI's i18n mechanism by exposing a new function `translate` directly from the `@neos-project/neos-ui-i18n` package.

The function looks as follows:
```typescript
function translate(
    fullyQualifiedTranslationAddressAsString: string,
    fallback: string | [string, string],
    parameters: Parameters = [],
    quantity: number = 0
): string;
```

And can be used like this:
```typescript
translate(
  'Neos.Neos:Main:workspaces.allChangesInWorkspaceHaveBeenDiscarded',
  'All changes from workspace "{0}" have been discarded.',
  ['user-admin']
);

// output (en): All changes from workspace "user-admin" have been discarded.
```

The new mechanism is completely independent from registries. There's no longer a need for injecting the `I18nRegistry` to use translations, instead `translate` can be used directly.

`translate` will only work after the translations have been loaded during the bootstrapping process of `@neos-project/neos-ui`. If it is used too early, it throws an exception.

The function now also properly supports plural rules (via [`Intl` Web API](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Intl)). Tha `I18nRegistry` had only supported English plurals, but with broken rules (quantity=0 yielded singular instead of plural).

Other changes included in this PR are:

- Everything in `@neos-project/neos-ui-i18n` is now TypeScript (save `manifest.js`)
- The `<I18n/>`-component was deprecated
- The `I18nRegistry` is now properly documented, but was also deprecated
- The `xliff.json` endpoint is now discovered via `<link>`-tag, instead of initial data
  - The `<link>`-tag is marked with `prefetch` to slightly speed up the fetch request
- The `@neos-project/neos-ui-i18n` now has a comprehensive README.md

This change is breaking, because translations may have relied on the broken plural rules. It is also breaking, because the signature for substitution parameters has changed slightly - although this should be non-breaking at runtime.
@mhsdesign
Copy link
Member

mhsdesign commented Jan 14, 2025

Okay after merging #3792 and #3804 i attempted to get this pr resolved of its conflicts. Now a rebase is a little cruel as there are lots of conflicts for the mentioned feature prs because this pr contains them too but in different commits. A merge worked just find but leaves a particular odd history obviously now. So im thinking of either squashing this pr when finished or work with some git magic to remove the other feature branch commits here ... or maybe dont take it too seriously like i do :D

Edit because the commits were split cleanly i was able to reset hard to 9.0 and just cherry pick all the changes you did after the original translation changes were made. That effectively drops the double commits. To confirm that the changes are correct i did a diff to my merge approach and even found an error in my merge resolution: (i had kept the loadImpersonateStatus function of f65f3cb) So now i pushed the rewrite and ist just the actual ca.20 commits instead of 60+ !!!

Aside from that i checked it out and fixed the linter and it compiles and from the looks of it works already. As this is wip there obviously still some todos in the code. Thanks again for your work as always!

grebaldi and others added 21 commits January 15, 2025 09:56
Until now, dropdown contents were `null`ed when the dropdown is closed.
This leads to unneccessary re-renders every time the dropdown visibility
is toggled.

This commit changes this logic. Dropdown contents are now always
rendered, but hidden when the dropdown is closed.
@mhsdesign mhsdesign force-pushed the feature/multi-app/02-loosen-the-menu branch from 9121cf6 to d1f2456 Compare January 15, 2025 09:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants