search

Helseatlas / helseatlas

Shiny-app to replace the current IA
MIT License
3 stars 2 forks source link

sorterbar tabell #146

Closed JaniceShu closed 4 years ago

JaniceShu commented 4 years ago

Using DT so that the table is sortable. Show all entries. Remove the numbers on the first column.

JaniceShu commented 4 years ago

There are still a couple small things I want to do on the tabell, so maybe don't delete the branch yet. Alternatively wait another day or two before merging the branch to master.

@arnfinn I don't think I've done a proper test. I have done 'Test package' and 'Check package' in R studio. What other tests should be done when we are making changes to the package??

arnfinn commented 4 years ago

No problem. We can wait.

The alternative would be to merge this into master, delete the branch, and then create a new branch based on master.

master has to be merged into this branch (and solving the conflicts) before this one can be merged into master. Do you want to make a try, or do you want me to do that?

All the testing is done by Travis and Appveyor, so in principle you do not have to test locally. Of course you have to make Travis and Appveyor pass the tests before you can merge it into master. Travis is failing because DT is not yet part of the docker image. I can make a quick fix (add a line to Dockerfile). The proper fix is to add DT to line 18 or 20 in this file: https://github.com/Helseatlas/helseatlas-base-r/blob/master/Dockerfile

EDIT: I made this PR that might help https://github.com/Helseatlas/helseatlas-base-r/pull/1

JaniceShu commented 4 years ago

@arnfinn I tried to resolve the conflict by merging with master to pick up changes to the module you and Tove have been working on (i.e. most recent master on github). Not sure if the way I did it makes it confusing. Basically the commits yesterday (12.5) can be ignored. But I noticed that the commits from today also picked up the changes to module, not just to tabell. Please have a look and see if it is ok, then just squash and merge then delete the branch. I will make a new branch when I make new improvements.

arnfinn commented 4 years ago

It looks good! I will merge it into master. Travis is failing because of too many requests to github or something. We can ignore it.