-
Notifications
You must be signed in to change notification settings - Fork 4
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
Use a different word for intervention per schema. #160 #271
Conversation
@@ -83,8 +84,8 @@ | |||
|
|||
{#if $currentMode == thisMode} | |||
{#if $formOpen} | |||
<p>Edit attributes to the left, or click another object</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I ditched "another", which maybe makes things less clear. Could have another variation of the singular noun function that omits the article, but was trying to avoid that :)
@@ -269,6 +275,6 @@ | |||
{:else if currentlyEditingControls == "route"} | |||
<RouteControls {routeTool} extendRoute={false} /> | |||
{:else} | |||
<p>Click an object to edit its geometry</p> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is definitely an improvement and allows us to be more specific. Nice!
@@ -92,3 +92,25 @@ export function schemaTitle(schema: Schema): string { | |||
atf4: "ATF4 Scheme", | |||
}[schema]; | |||
} | |||
|
|||
// The singular form of the object (with the article) managed by this schema | |||
export function schemaSingularNoun(schema: Schema): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
So nice! I wonder if we could read this in from a .json config file to separate configuration from code as we did for the schemea.json files?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We could put these and the title in the same .json defining the schema, yes. Maybe it does fit nicely there. OK if we think about that and defer a refactor? Still unsure how much we want to invest in this approach generally, especially now that we're running against govuk style best practices with the expanding form so much.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
OK if we think about that and defer a refactor?
👍
Yes this is a nice to have from my perspective.
} | ||
|
||
// The plural form of the object managed by this schema | ||
export function schemaPluralNoun(schema: Schema): string { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Gota have the plural versions also! They look right to me.
See #160.
I think I got the tests, but slow internet at the moment, so can't run easily