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

DOC Document localising links #273

Conversation

GuySartorelli
Copy link
Member

@GuySartorelli GuySartorelli commented Apr 10, 2024

Description

Documents localising links with fluent.

Note that this won't work unless silverstripe/silverstripe-framework#11195 is also merged. You can work around that by adding this configuration:

SilverStripe\LinkField\Models\Link:
  extensions:
    - TractorCow\Fluent\Extension\FluentVersionedExtension
  field_exclude:
    - OwnerRelation

Original fluent spike

Issues

Copy link
Member

@emteknetnz emteknetnz left a comment

Choose a reason for hiding this comment

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

I had a quick test of this after referencing my notes in the original spike

While has_many looks like it works because, it really doesn't

  • Sorting has_many in one locale will unexpectedly sort the links in the other locale which is very unexpected
  • Archiving a link does nothing. Try refreshing the page after deleting it.

I stopped testing at this point. I think it's simply a case that "has_many doesn't work"

@GuySartorelli
Copy link
Member Author

GuySartorelli commented Apr 10, 2024

Sorting has_many in one locale will unexpectedly sort the links in the other locale which is very unexpected

I disagree - I think that is very expected. I think in most cases you would want the links in the same order for all locales and only change the text. Imagine having to go through all of your locales every time you want to change the order of links - what a nightmare!

If developers want to sort the links differently per locale, that is doable but goes beyond the basic scenario I have documented here - people who want that probably are already doing that kind of thing elsewhere in their project as well - but if not, they can read the fluent docs to look for information about how fluent works.

I want to avoid duplication of documentation as much as possible.

Archiving a link does nothing. Try refreshing the page after deleting it.

It archives the specific version you have in that locale. After you refresh, you see the version of the link which is stored in the fallback locale. Notice the title of the link is different before and after archiving in this video:

Screencast.from.10-04-24.16.19.02.webm

There are a couple of UX issues here:

  1. The link shouldn't disappear after archiving - it should just immediately show the copy of the link from the fallback locale
  2. There should be fluent labels on the links indicating where the link information is coming from (so it's clear that version is not from this locale, it's from the fallback)
    • Obviously we shouldn't add coupling here - but we could provide an API for adding arbitrary labels that fluent or some intermediary "compatibility" module could take advantage of
  3. The archive option shouldn't be available when the link hasn't been saved in the current locale

Those UX issues are not in the scope of this PR, which is just adding documentation. If you want, I can document these UX issues.

I think it's simply a case that "has_many doesn't work"

I disagree. As stated above, it seems like there are just a few UX issues that need to be ironed out.
The archiving UX problem is not specific to has_many either - that's the behaviour for all links, even has_one.

@emteknetnz
Copy link
Member

If we want to say that fluent works correctly, though there's some caveats, and we need to do more work on linkfield to get fluent to work as you think it should, then it's too soon to merge these docs as they are where "fluent has_many works" where IMO it really doesn't

I think this one needs to go back in refinement for discussion to work out what we want to do with fluent since it sounds like there's a bunch more work to do.

@GuySartorelli
Copy link
Member Author

You're focusing a lot on has_many but the only outstanding issues seem to be around archiving, which as I noted affect has_one as well.
In any case, I've put the issue back into refinement and I'll close this PR.

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.

2 participants