Show manual frequency in connection table and use intl for frequency values #13621
Conversation
Add getFrequencyConfig utility to correctly find frequency configuration
| { | ||
| id: `frequency.${ | ||
| item.config.timeUnit === "hours" && item.config.units === 1 ? "hour" : item.config.timeUnit | ||
| }`, |
There was a problem hiding this comment.
I see that you have some logic for singular/plural in the en.json... are the "hour" entry and this logic needed with the singular/plural logic you have for "hours"?
There was a problem hiding this comment.
Sometimes we want to display "Every hour" (in the replication settings) but in other cases we display "1 hour" (in the connections table). Maybe I should replace the "frequency.hour" key to be more clear?
There was a problem hiding this comment.
Could we maybe replace the form.every with two different keys we're using here instead for form.every.hours and form.every.minutes. that way we could use the pluralization inside en.json to take care of this, which imho would read a bit nicer (and would make it easier for translators later).
There was a problem hiding this comment.
@timroes I was thinking the same thing because it made this a little too convoluted.
|
@tealjulia There was one more thing I needed to fix in regards to showing "Manual" as the frequency in the Replication settings. It was a very small fix so I included it as part of this change since it also fixes the issue where "Manual" was not showing. |
Signed-off-by: Edmundo Ruiz Ghanem <[email protected]>
edmundito
force-pushed
the
edmundito/show-manual-frequency
branch
from
8821e15 to
6291021
Compare
What
When the connection frequency is set to manual, it now shows in the connection table:
And when the replication settings, except for when it's a new connection:
The frequency values were not part of intl so they were also moved there.
How
The reason why it wasn't showing before was a missed comparison between
undefinedandnull. This comparison is now made when getting the frequency with the newgetFrequencyConfigutil, which is mostly used to track events.This issue may have been introduced with #12071 because some of the types changed there and what was once set to
nullmay now be set toundefined, andnull !=== undefinedFor the actual display of the frequency now it's using strings from the
en.jsonfile instead of what was hard-coded in the config. In order to avoid confusion, thetextproperty in the config was renamed totype.Recommended reading order
Top to bottom