Skip to content

Editable reports, Centralized settings - #780

Merged
fvacek merged 24 commits into
masterfrom
centralized-settings
Sep 10, 2022
Merged

fvacek merged 24 commits into
masterfrom
centralized-settings

Conversation

@fvacek

@fvacek fvacek commented Jul 17, 2022 •

Copy link
Copy Markdown
Collaborator

Vratit reporty z resources do adresaru, kde je QE vyhledava #722

zcentralizoval jsem settings (az na nastaveni receipts)

image

  1. kliknutim na button create se vyrobi kopie reportu z resources ve vybranem adresari,
  2. v custom reports dir si kazdy muze upravovat, co chce
  3. pokud soubor v adresari jiz existuje, kliknuti na create ho neprepise, tim se zabrani nechtenemu prepsani poeditovanych reportu
  4. pokud si nekdo v adresari receipts nebo awards vytvori dalsi soubory, budou se mu nabizet v listboxech pro vyber ustrizku/diplomu vedle tech z resources
  5. je tam docela dost uprav, hodne jsem to proklikal, ale mozna jsem neco rozbil.

@fvacek
fvacek requested review from otahirs and paukert July 17, 2022 20:33
@fvacek fvacek changed the title Centralized settings Editable reports, Centralized settings Jul 17, 2022
@fvacek
fvacek requested a review from arnost00 July 17, 2022 20:47

@arnost00 arnost00 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Only one small thing in UI, mising bottom Spacer in Settings->CardReader
Qml reports exports fine on Windows.

@otahirs

otahirs commented Jul 21, 2022

Copy link
Copy Markdown
Member

I like the idea of centralized settings, however there cant be a guesswork where to find the settings. As for now, everything was in modules, if we want to change it, everything (that is not meant to change during the event) should move to settings.
-> move receipts settings
-> move Card reader check type
-> Cards that can be rented (cards table) could also find a place there?

Reports

  1. Create button opens folder selection the same way as the ... button. I guess it should only create the files (or remove the ... button).
  2. the files are hidden in all folders under /qml/reports/, i find this redundant
  3. I would write the following guide under the select box ⬇️

kliknutim na button create se vyrobi kopie reportu z resources ve vybranem adresari,
v custom reports dir si kazdy muze upravovat, co chce
pokud soubor v adresari jiz existuje, kliknuti na create ho neprepise, tim se zabrani nechtenemu prepsani poeditovanych reportu
pokud si nekdo v adresari receipts nebo awards vytvori dalsi soubory, budou se mu nabizet v listboxech pro vyber ustrizku/diplomu vedle tech z resources

Card reader

  1. typo
    image
  2. there should be a test connection button, so I dont have to switch between Open COM in Card reader module and the settings

fvacek added 2 commits July 21, 2022 14:10
# Conflicts:
#	libquickevent/libquickeventcore/src/exporters/htmlfileexporter.cpp
#	quickevent/app/quickevent/plugins/Relays/src/relayswidget.cpp
@fvacek

fvacek commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator Author

Only one small thing in UI, mising bottom Spacer in Settings->CardReader Qml reports exports fine on Windows.

will be fixed

@fvacek

fvacek commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator Author

I like the idea of centralized settings, however there cant be a guesswork where to find the settings. As for now, everything was in modules, if we want to change it, everything (that is not meant to change during the event) should move to settings.

agree

move receipts settings

agree

move Card reader check type

agree

Cards that can be rented (cards table) could also find a place there?

hard to say, correct place for this is difficult to find, if yes, which section?

@fvacek

fvacek commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator Author

Reports

`Create` button opens folder selection the same way as the `...` button. I guess it should only create the files (or remove the `...` button).

What if you have user reports created from previous installation already and you cast want to select their dir? Current implementation doesn't override any existing file, so create will behave in the same way, like ..., but might be confusing for user.
More than that, the user reports dir is overlay over resources, so you can have only new or changed reports here.

the files are hidden in all folders under /qml/reports/, i find this redundant

there are more files under qml than just reports in some plugins, so reports is important to mark dirs to export

I would write the following guide under the select box arrow_down

agree

@fvacek

fvacek commented Jul 25, 2022

Copy link
Copy Markdown
Collaborator Author

Card reader

1. typo
   ![image](https://user-images.githubusercontent.com/24588500/180160612-3bb026f9-0bb7-43d3-91c0-3563271ff9e0.png)

missing buddy link, will be fixed

there should be a test connection button, so I dont have to switch between Open COM in Card reader module and the settings

agree

@paukert

paukert commented Jul 27, 2022

Copy link
Copy Markdown
Member

I went quickly through changes and it looks fine overall. Some improvements were already suggested by @otahirs. I will check changes in more depth next week.

@fvacek
fvacek marked this pull request as draft August 7, 2022 19:19
@otahirs

otahirs commented Aug 29, 2022

Copy link
Copy Markdown
Member

Test connection pops up OK even though connection is not estabilished

@fvacek

fvacek commented Aug 29, 2022

Copy link
Copy Markdown
Collaborator Author

strange, this is how it works on by box, maybe windows hell?
image

@otahirs

otahirs commented Aug 29, 2022 •

Copy link
Copy Markdown
Member

in case some device exist on the path, but is not a valid Card reader
image

@fvacek

fvacek commented Aug 29, 2022

Copy link
Copy Markdown
Collaborator Author

Now I understand, port is open OK, but it is not an SI reader.

@otahirs

otahirs commented Aug 29, 2022 •

Copy link
Copy Markdown
Member

new Check type and Reader mode options are appended every time the Card reader tab is selected
image


In Receipts settings, Make sure the Default (or Classic) Receipt is selected by default, and not the Classic lottery

@fvacek

fvacek commented Aug 29, 2022

Copy link
Copy Markdown
Collaborator Author

I do not have SI reader today, so SI reader connect test might not work as expected. Thanks for testing 👍

@otahirs

otahirs commented Aug 29, 2022

Copy link
Copy Markdown
Member

Reports still missing guide on "How to use".
also the label is quite long, maybe put the input on a new line
image


I feel like "Settings" should be a place i visit at the start of the event and do not touch later. It is that reason, i am not sure toggle between readout and edit on punch belong here. Just a thought.
image


also print new button seems not intuitive when auto print box is not present next to it. maybe autoprint could also stay in the module.

@otahirs

otahirs commented Aug 29, 2022

Copy link
Copy Markdown
Member

This still feels suboptimal. Previously, clicking the open port button led straight to an error.
2022-08-29 23-05-08

@otahirs

otahirs commented Aug 30, 2022

Copy link
Copy Markdown
Member

I have not checked the recent commit, just a reminder, that in verison 2.4.6 the outcome of connection was known imediately

2022-08-30.10-53-42.mp4

@otahirs

otahirs commented Aug 30, 2022

Copy link
Copy Markdown
Member
  1. Event edit can also be moved to settings -> Event
  2. I also think that Check type - Classic race | Free order race should be moved into this form
    image

@otahirs

otahirs commented Aug 30, 2022

Copy link
Copy Markdown
Member

Cards that can be rented (cards table) could also find a place there?

hard to say, correct place for this is difficult to find, if yes, which section?

I would create Cards to rent tab and kind of just inline the Cards to rent dialog there

@fvacek
fvacek marked this pull request as ready for review September 10, 2022 17:24
@fvacek
fvacek merged commit 08baf71 into master Sep 10, 2022
@fvacek

fvacek commented Sep 10, 2022

Copy link
Copy Markdown
Collaborator Author

I have to marge this PR to prevent big branch diversion with master. Serious issues are fixed, I'll create new issues for minor issues and suggestions.

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.

4 participants