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

Configurable language #1236

Merged

Conversation

morpheus-87
Copy link
Contributor

This PR makes the language of the UI configurable in the init step.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 61.797% when pulling a3f3eaa on dbmdz:feature/configurable-language into b4248b4 on ProjectMirador:2.1.3.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 61.797% when pulling 9e223d3 on dbmdz:feature/configurable-language into 099324d on ProjectMirador:2.1.3.

@rsinghal rsinghal changed the base branch from 2.1.3 to 2.1.4 February 9, 2017 16:32
@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 61.474% when pulling 7f0eb4c on dbmdz:feature/configurable-language into 993b08e on ProjectMirador:2.1.4.

@aeschylus
Copy link
Collaborator

This has some conflicts with the improvements from one of your other PRs. Also, there is another way to implement configuration which is used in other areas of the app, so can you resubmit this as a new PR with that pattern?

A good example can be found by looking at the saveController.js and the DEFAULT_SETTINGS in settings.js

Marking as wontfix for now, but open to a revised PR.

@morpheus-87
Copy link
Contributor Author

I checked your change request and saw, that there is no other way to get configuration parameters in viewer.js except reading it from the state.

It seems, that the behaviour you are talking about, was removed in commit d0f6310 by @rsinghal.

@rsinghal
Copy link
Collaborator

rsinghal commented Mar 2, 2017

That's the correct way to get information from the configuration @aeschylus. Not sure what you are looking for?

@rsinghal rsinghal changed the base branch from 2.1.4 to 2.3.0 March 2, 2017 15:17
@rsinghal rsinghal reopened this Mar 2, 2017
@aeschylus aeschylus removed the wontfix label Mar 2, 2017
@aeschylus
Copy link
Collaborator

Sorry, was closing like 12 PRs simultaneously and don't think I grasped what this was trying to do. If you need to get the configuration dynamically (rather than on initialisation of the whole App using extend), this makes sense.

Can you settle the conflict in viewer.js and we can merge from there?

@morpheus-87 morpheus-87 force-pushed the feature/configurable-language branch from 7f0eb4c to e27cd27 Compare March 3, 2017 07:58
@coveralls
Copy link

Coverage Status

Coverage increased (+0.05%) to 61.7% when pulling e27cd27 on dbmdz:feature/configurable-language into ea2a0a5 on ProjectMirador:2.3.0.

@morpheus-87
Copy link
Contributor Author

morpheus-87 commented Mar 3, 2017

In the meantime, I fixed the conflict, @aeschylus.

I would like to add a testcase for this functionality, what would be the best place to do this?

@aeschylus
Copy link
Collaborator

Looks like viewer.js is still a stub, but that would be the place I think. You would need to test that the language was one thing in the config before the runtime initialisation.

Thanks for fixing this up. Looking forward to merging.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 61.559% when pulling 40cfdd5 on dbmdz:feature/configurable-language into 2d7c9cb on ProjectMirador:2.3.0.

@morpheus-87
Copy link
Contributor Author

morpheus-87 commented Mar 6, 2017

Is it really necessary to have a separate testcase for setting the language, @aeschylus?

@aeschylus
Copy link
Collaborator

@morpheus-87 Always a fair question. I think it could be useful since it's happening at runtime. Can one of us help set up the test case?

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 62.037% when pulling 09bbf69 on dbmdz:feature/configurable-language into 4522310 on ProjectMirador:2.3.0.

@coveralls
Copy link

Coverage Status

Coverage increased (+0.01%) to 62.037% when pulling 05401c9 on dbmdz:feature/configurable-language into 50b1730 on ProjectMirador:2.3.0.

@morpheus-87
Copy link
Contributor Author

Would be great, if somebody could help me with setting up the test case.

@coveralls
Copy link

coveralls commented Mar 24, 2017

Coverage Status

Coverage increased (+0.07%) to 62.117% when pulling 90239ea on dbmdz:feature/configurable-language into 37c3062 on ProjectMirador:2.3.0.

@morpheus-87 morpheus-87 changed the base branch from 2.3.0 to 2.4.0 March 27, 2017 08:31
@coveralls
Copy link

coveralls commented Mar 27, 2017

Coverage Status

Coverage increased (+0.01%) to 62.059% when pulling 4a102f7 on dbmdz:feature/configurable-language into 632bcce on ProjectMirador:2.4.0.

@morpheus-87
Copy link
Contributor Author

Could somebody help me with setting up the test case?

@coveralls
Copy link

coveralls commented Apr 4, 2017

Coverage Status

Coverage increased (+0.01%) to 62.059% when pulling b5eabe6 on dbmdz:feature/configurable-language into 2d4a908 on ProjectMirador:2.4.0.

@coveralls
Copy link

coveralls commented Apr 5, 2017

Coverage Status

Coverage increased (+0.01%) to 62.059% when pulling 4bc0e04 on dbmdz:feature/configurable-language into 537b037 on ProjectMirador:2.4.0.

@aeschylus aeschylus changed the base branch from 2.4.0 to develop April 12, 2017 20:55
@morpheus-87 morpheus-87 force-pushed the feature/configurable-language branch from 4bc0e04 to 69c72e7 Compare April 18, 2017 08:03
@coveralls
Copy link

coveralls commented Apr 18, 2017

Coverage Status

Coverage increased (+0.01%) to 62.027% when pulling 69c72e7 on dbmdz:feature/configurable-language into bf3eb06 on ProjectMirador:develop.

@morpheus-87 morpheus-87 force-pushed the feature/configurable-language branch from 69c72e7 to 451ccb7 Compare April 25, 2017 08:12
@coveralls
Copy link

coveralls commented Apr 25, 2017

Coverage Status

Coverage decreased (-0.05%) to 61.972% when pulling 451ccb7 on dbmdz:feature/configurable-language into 529380b on ProjectMirador:develop.

@rsinghal
Copy link
Collaborator

rsinghal commented May 1, 2017

@coveralls
Copy link

coveralls commented May 1, 2017

Coverage Status

Coverage increased (+0.06%) to 62.027% when pulling 94e8994 on dbmdz:feature/configurable-language into 52b572e on ProjectMirador:develop.

@coveralls
Copy link

coveralls commented May 1, 2017

Coverage Status

Coverage decreased (-0.05%) to 61.972% when pulling 528beea on dbmdz:feature/configurable-language into 4cfa692 on ProjectMirador:develop.

@morpheus-87 morpheus-87 force-pushed the feature/configurable-language branch from fed5a69 to 96b3df2 Compare June 2, 2017 10:04
@coveralls
Copy link

coveralls commented Jun 2, 2017

Coverage Status

Coverage increased (+0.02%) to 72.537% when pulling 96b3df2 on dbmdz:feature/configurable-language into 918a539 on ProjectMirador:develop.

@morpheus-87
Copy link
Contributor Author

I just updated the branch and set up a testcase in spec/mirador.test.js.

@coveralls
Copy link

coveralls commented Jun 9, 2017

Coverage Status

Coverage increased (+0.1%) to 72.618% when pulling f8ed3d0 on dbmdz:feature/configurable-language into 9ae2cf9 on ProjectMirador:develop.

@aeschylus
Copy link
Collaborator

Thanks @morpheus-87. This looks good to me.

@aeschylus aeschylus merged commit 27fae8e into ProjectMirador:develop Jun 9, 2017
@morpheus-87 morpheus-87 deleted the feature/configurable-language branch June 10, 2017 17:34
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.

None yet

5 participants