DT-4910: New texts for infobox #4875

Dvun · 2023-10-23T12:29:18Z

Proposed Changes

New texts for infobox

Pull Request Check List

A reasonable set of unit tests is included
Console does not show new warnings/errors
Changes are documented or they are self explanatory
This pull request does not have any merge conflicts
All existing tests pass in CI build
Code coverage does not decrease (unless measured incorrectly)

Review

Read and verify the code changes
Test the functionality by running the UI locally with all popular browsers available in your platform
Check that the implementation matches the design, when such one is defined in a Jira issue
Merge the pull request

sharhio · 2023-10-24T12:26:08Z

app/translations.js

@@ -1295,6 +1295,8 @@ const translations = {
    'number-of-minutes':
      '{number, plural, =0 {0 minutes} one {1 minute} other {{number} minutes}}',
    'number-of-spaces': 'Number of spaces:',
+    'nysse-ticket-limited':
+      'Nysse tickets are valid on trains in the Nysse area with some limitations. Read more on ',


Tekstin tulisi olla geneerinen nimeltään tyyliin 'train-ticket-limited' ja "Nysse"n voisi korvata muuttujalla tyyliin {{agencyName}}. Tai koko teksti tampereen config tiedostoon.

sharhio · 2023-10-24T12:26:34Z

app/translations.js

@@ -2431,6 +2433,8 @@ const translations = {
    'number-of-minutes':
      '{number, plural, =0 {0 minuuttia} one {1 minuutti} other {{number} minuuttia}}',
    'number-of-spaces': 'Paikkojen määrä:',
+    'nysse-ticket-limited':
+      'Nyssen liput käyvät junaliikenteessä rajoitetusti vain Nysse-alueella. Lue lisää ',


Sama kuin ylempänä.

sharhio · 2023-10-24T12:26:51Z

app/translations.js

@@ -4347,6 +4351,8 @@ const translations = {
    'number-of-minutes':
      '{number, plural, =0 {0 minuter} one {1 minut} other {{number} minuter}}',
    'number-of-spaces': 'Antalet platser:',
+    'nysse-ticket-limited':
+      'Nysse-biljetter är giltiga på tåg i Nysse-området, med vissa begränsningar. Läs mer på ',


Sama kuin ylempänä.

sharhio · 2023-10-24T12:52:54Z

app/component/TransitLeg.js

+          {leg.fare &&
+            leg.fare.isUnknown &&
+            shouldShowFareInfo(config) &&
+            (config.showTrainLimitationInfo ? (


Tälle parempi sijainti voisi olla RailLeg, samaan tyyliin kuin CallAgencyLeg pitää oman infoboksinsa sisällään.

Tarkemmin katsottuja RailLeg ei ole samantyyppinen kuin callAgencyLeg ja pitääkin sisällään TransitLegin ja määrittelee sille junan parametrejä, eli lokaatio on oikea. Tarkistuksen voisi kuitenkin lisätä että kyseessä on junaliikenne, joka onnistunee mode muuttujan avulla.

mode === 'RAIL' on lisätty, mutta tämä on vain lisä turha koodi lisämine sinne. Koska oli testattu kävelly, bussin, polkupyörä, lentokonen matkat ja vain junan kanssa infoteksti tulee sinne jos tarvitaessa.

sharhio · 2023-10-24T13:03:13Z

app/component/ItineraryTab.js

+        );
+      }
+
+      if (config.showTrainLimitationInfo) {


Tässä olisi hyvä varmistaa, että ehdotukset sisältävät junaliikennettä (esim. samaan tyyliin kuin yllä kutsuliikenteellä itineraryContainsCallLegs).

sharhio · 2023-11-08T07:30:50Z

app/util/legUtils.js

@@ -34,6 +34,13 @@ export function isCallAgencyPickupType(leg) {
  );
 }

+export function isTrainLimitationPickupType(leg) {
+  return (
+    filterLegStops(leg, stoptime => stoptime.pickupType === 'SCHEDULED')


Junia ei voi tarkistaa pickupTypestä, koska muillakin kuin junilla on scheduled pickuptype. Legin modesta tiedon kyllä saa ("rail").

vesameskanen · 2023-11-15T10:47:04Z

app/component/InfoBox.js

+import Icon from './Icon';
+
+const InfoBox = (
+  { textId, values, href = null, configData = null },


Component name cannot be generic InfoBox, because it renders very specific fare disclaimer .

FareDisclaimer.js

vesameskanen · 2023-11-15T10:57:50Z

app/component/ItineraryTab.js

+    const itineraryContainsCallLegs = itinerary.legs.some(leg =>
+      isCallAgencyPickupType(leg),
+    );
+    const isTrainLimitation = itinerary.legs.some(leg =>


Poor naming:

isTrainLimitation -> hasTrainLegs. Actually this could be more generic: configuration could include a map of disclaimers per mode.

isShowItineraryContainsCallLegsInfo is totally incomprehensible, change it to 'showCallAgencyDisclaimer'

isShowTrainLimitationInfo -> showLegModeDisclaimer

vesameskanen · 2023-11-15T11:00:08Z

app/util/legUtils.js

+  );
+}
+
+export function isTrainLimitationPickupType(leg) {


Bad name. This has nothing to do with pickup types.

vesameskanen · 2023-11-15T11:24:35Z

app/configurations/config.tampere.js

+        'Trains in the Nysse area - Nysse, Tampere regional transport',
+    },
+  },
+


modeDisclaimers: {
train: {
fi: {
showTrainLimitationInfoLink: 'https://www.nysse.fi/junat',
showTrainLimitationInfoLinkText: 'nysse.fi/junat',
},
sv: {
showTrainLimitationInfoLink:
'https://www.nysse.fi/en/ways-to-get-around/train',
showTrainLimitationInfoLinkText:
'Trains in the Nysse area - Nysse, Tampere regional transport',
},
en: {
showTrainLimitationInfoLink:
'https://www.nysse.fi/en/ways-to-get-around/train',
showTrainLimitationInfoLinkText:
'Trains in the Nysse area - Nysse, Tampere regional transport',
},
},
}
}

vesameskanen · 2023-11-16T07:15:34Z

app/configurations/config.tampere.js

+      fi: {
+        showTrainLimitationInfoLink: 'https://www.nysse.fi/junat',
+        showTrainLimitationInfoLinkText: 'nysse.fi/junat',
+      },


Muuta showTrainLimitationInfoLink -> link, showTrainLimitationInfoLinkText -> text

vesameskanen · 2023-11-16T07:17:01Z

app/component/TransitLeg.js

+            leg.fare.isUnknown &&
+            shouldShowFareInfo(config) &&
+            (mode === LegMode.Rail && config.modeDisclaimers.rail ? (
+              <div className="disclaimer-container unknown-fare-disclaimer__leg">


Koeta koodata tämä näin:

(config.modeDisclaimers[mode] ?

Suora käytä mode en pysty, koska tämä tulee kun 'RAIL', mutta tarvitseen configille 'rail'.
Onko sitten parempi käytää tällä mode.toLowerCase() tai sitten sama tiedostossa oli jo aikasemmin kirjoitety
const modeClassName = mode.toLowerCase();
Tämä on hyvä käytä, mutta tuntuu, että muttoja nimi modeClassName ei ole hyvä ???

vesameskanen · 2023-11-16T07:18:11Z

app/util/legUtils.js

+}
+
+export function hasLegMode(leg) {
+  return getLegMode(leg) === LegMode.Rail;


Tässä funktiossa ei ole järkeä. Poista ja siirrä yhden rivin koodi ItineraryTab.js:lle

vesameskanen · 2023-11-16T07:27:06Z

app/component/ItineraryTab.js

+      isCallAgencyPickupType(leg),
+    );
+    const hasTrainLegs = itinerary.legs.some(leg => hasLegMode(leg));
+


Seuraava olisi hyvä ratkaisu:
Rakenna taulukko FareDisclaimereja. Taulukoon pusketaan CallAgency yms disclaimerit, ja lisäksi n kpl mode disclaimereja. Eli iteroi kaikki legit ja katso onko käytössä leg.mode disclaimer. Jos on puske taulukkoon.
Tekstit kannattaa siirtää pois translations.txt tiedostosta config .modeDisclaimer objektiin.

vesameskanen

Muutokset menee oikeaan suuntaan, mutta vieläkin voi parantaa.

Dvun added 8 commits October 20, 2023 11:15

fix: added new links for Tampere

9846088

fix: added new text in infobox with links

33b3c11

fix: added new text in infobox with links

182ba1c

fix: added new translation texts

67fc9ae

fix: renamed names in config

99f7b31

fix: renamed

e807d10

fix: added new text for infobox

290cf17

fix: added new text for infobox

7cb8c67

sharhio requested changes Nov 1, 2023

View reviewed changes

Dvun added 3 commits November 1, 2023 15:31

fix: renamed nysse-ticket-limited" to train-ticket-limited"

f569001

fix: new function for check pickup type

ed2ca98

fix: added mode check

3cb02df

Dvun force-pushed the DT-4910 branch from 774fbb5 to 3cb02df Compare November 7, 2023 13:40

sharhio reviewed Nov 8, 2023

View reviewed changes

Dvun added 10 commits November 8, 2023 10:17

fix: fixed function to check is Rail mode or not

6cb1f7f

Merge branch 'v3' into DT-4910

22ed412

fix: fixed mystic string to constants

08845a0

fix: fixed check

acad2d2

fix: removed not needed if

ca55d23

fix: totally reworked infoBoxText and created with nested constants

81dfc4b

fix: if statement fix and removed 2 constants for default infoText

ac037fb

fix: created new InfoBox component

ae21227

fix: infoboxtext moved to own component

28d9113

fix: fixed if statement for info box

2487893

vesameskanen reviewed Nov 15, 2023

View reviewed changes

Dvun added 4 commits November 15, 2023 14:22

fix: renamed from InfoBox to FareDisclaimer + added defaultProps

9c720c7

fix: config method renamed

14043e3

fix: renamed config method

291099d

fix: renamed function name

798ee74

vesameskanen reviewed Nov 16, 2023

View reviewed changes

Dvun and others added 23 commits November 16, 2023 15:32

fix: config update

8934ca8

fix: renamed configData to linkText

0954c57

fix: reworked, updated function to show info text

44d211b

fix: removed unnecessary function

c5d341d

fix: updated to more generic

dafbba7

fix: fix

f3274b1

fix: fixed same text pushed many times

1c1b6e1

fix: fix code

f575303

fix: code fix

8ee65b8

fix: code fix

88aad96

fix: removed config

84758f0

fix: updated code and config with agencyName coming inside values

d8faf24

fix: removed 'train-ticket-limited' texts

a5bec27

fix: update to more generic properties

d94c436

fix: update to more generic properties

f1c717e

fix: update to more generic properties

125255c

fix: moved texts from translation.js

bc3c5a5

fix: fix

7b77158

fix: bad update :(

b9c7ed1

fix: changed propTypes

0285ffa

fix: update

8b6c417

chore: remove redundant variables

65567b7

fix: incorrect message id

74e5f3c

vesameskanen merged commit a7d3bd2 into v3 Nov 20, 2023
5 of 6 checks passed

vesameskanen deleted the DT-4910 branch November 20, 2023 06:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DT-4910: New texts for infobox #4875

DT-4910: New texts for infobox #4875

Dvun commented Oct 23, 2023

sharhio Oct 24, 2023

sharhio Oct 24, 2023

sharhio Oct 24, 2023

sharhio Oct 24, 2023

sharhio Nov 7, 2023

Dvun Nov 7, 2023

sharhio Oct 24, 2023

sharhio Nov 8, 2023

vesameskanen Nov 15, 2023

vesameskanen Nov 15, 2023

vesameskanen Nov 15, 2023

vesameskanen Nov 15, 2023

vesameskanen Nov 15, 2023

vesameskanen Nov 16, 2023

vesameskanen Nov 16, 2023

Dvun Nov 16, 2023

vesameskanen Nov 16, 2023

vesameskanen Nov 16, 2023 •

edited

Loading

vesameskanen left a comment

DT-4910: New texts for infobox #4875

DT-4910: New texts for infobox #4875

Conversation

Dvun commented Oct 23, 2023

Proposed Changes

Pull Request Check List

Review

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

vesameskanen Nov 16, 2023 • edited Loading

Choose a reason for hiding this comment

vesameskanen left a comment

Choose a reason for hiding this comment

vesameskanen Nov 16, 2023 •

edited

Loading