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

Matrix view #9

Open
wants to merge 18 commits into
base: develop
Choose a base branch
from
Open

Matrix view #9

wants to merge 18 commits into from

Conversation

flo80
Copy link
Contributor

@flo80 flo80 commented Jul 18, 2019

Added a new matrix view (i.e. one line per destination with several departure times)

Screenshot:

fewieden and others added 18 commits March 22, 2017 11:31
* Added WienerLinien disruption info (lines and elevators) (fewieden#3)

* Added functions to display interuptions to lines and elevators

* Added .gitignore from MM

* Fixed typos in README.md

* Changed line disruptions text to full description

* removed unused property

* Fixed lint errors

* Fixed README for config parameter name change

* removed project unrelated gitignores

* github templates

* translations, changelog and cleanup of pr fewieden#3
@fruestueck
Copy link

I really like the looks of it!

Copy link

@fruestueck fruestueck left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall I really like the matrix view.

It would also give us the possabillity to aggregate all stations into one view without rotating between them later (I'm only using three stations).

/* eslint-disable func-names */
/* eslint-disable prefer-arrow-callback */
/* eslint-disable no-var */
/* eslint-disable vars-on-top */

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what impact does this have?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This just disables some linting warnings, you can get rid of it, then you will see a few warnings

Comment on lines -204 to +300
for (let i = 0; i < this[type].length; i += 1) {
for (var i = 0; i < this[type].length; i += 1) {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why do we move to var from let?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am using an old ipad as magic mirror device. And the old safari version does not work with let, this is why I use vars. (on newer browsers it also works with let)

Comment on lines -232 to +329
let temp = text;
var temp = text;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here (var)?

newline not necessary. Style thing thou.

Comment on lines -7 to +11
![](.github/example.jpg) ![](.github/example2.jpg) ![](.github/example3.PNG)
![](.github/example.jpg) ![](.github/example2.jpg) ![](.github/example3.PNG) ![](.github/example4.png)


Matrix view
![](.github/example-matrix.png)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added images are to big.

I'd also reduce the amount of images. Maby swap example-matrix with example2. Should be enoth.

@@ -37,7 +41,8 @@ Public Transport of Vienna/Austria Module for MagicMirror<sup>2</sup>
| --- | --- | --- |
| `api_key` | REQUIRED | Get an API key for free access to the data of www.wienerlinien.at [here](https://www.wien.gv.at/formularserver2/user/formular.aspx?pid=3b49a23de1ff43efbc45ae85faee31db&pn=B0718725a79fb40f4bb4b7e0d2d49f1d1). |
| `stations` | REQUIRED | Insert here the station ids you want to display data from [How to find an ID?](https://till.mabe.at/rbl/). |
| `max` | `5` | How many departures should be displayed. |
| `display` | `list` | Show departures as list or as `matrix` (see screenshots). |

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Both options - matrix and list -should be highlighted.

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

3 participants