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

correctly convert date/time format to respect field and display formats #421

Merged
merged 11 commits into from Jan 29, 2019

Conversation

Projects
None yet
4 participants
@3nids
Copy link
Member

3nids commented Dec 18, 2018

these formats are defined in QGIS and can differ from each other
more over, the type of value from the field can be either a date or a string

by correctly constructing a date/time object when getting/setting the value from
the widget, the same behavior than in QGIS is allowed

also:

  • do not open calendar when option is not checked in QGIS project
  • define inputMask whenever easily possible

fix #9
fix #215
fix #371

supersedes #404

@3nids 3nids requested a review from m-kuhn Dec 18, 2018

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Dec 18, 2018

@saemiw would be good to get your approval here

@qfield-fairy

This comment has been minimized.

Copy link
Collaborator

qfield-fairy commented Dec 18, 2018

Uploaded test apks for armv7 and x86

@saemiw

This comment has been minimized.

Copy link

saemiw commented Dec 19, 2018

@3nids everything looks fine upon data entry. The data are however not saved. When I revisit a feature that I already entered before I can't see the entered time. AND when I try to modify the time it will not accept my entries (field remains empty). The only time that is accepted and written to the geopackage file is when it is entered via a now()-default-value.

Entry before saving anything
screenshot_20181219-083829_qfield

This is what is saved when re-visiting the feature
screenshot_20181219-083852_qfield

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Dec 19, 2018

@saemiw thanks a lot for the report. I had only tested on editing on current features, and I thought it was working while not.
It seems the widget doesn't update itself properly (keep values of the first open feature).
Still on progress

@qfield-fairy

This comment has been minimized.

Copy link
Collaborator

qfield-fairy commented Dec 19, 2018

Uploaded test apks for armv7 and x86

@saemiw

This comment has been minimized.

Copy link

saemiw commented Jan 7, 2019

I did a quick test on the latest test apk. Data entry now seems to work fine. But when I modifie (change) a time-entry the attribute value is changed in all features instead of just the one that I wanted to modify.

@@ -128,6 +128,7 @@ QVariant FeatureModel::data( const QModelIndex& index, int role ) const
break;

case AttributeValue:
qDebug() << mFeature.attribute( index.row() );

This comment has been minimized.

@m-kuhn

m-kuhn Jan 7, 2019

Member

Remove

3nids added some commits Dec 7, 2018

correctly convert date/time format to respect field and display formats
these formats are defined in QGIS and can differ from each other
more over, the type of value from the field can be either a date or a string

by correctly constructing a date/time object when getting/setting the value from
the widget, the same behavior than in QGIS is allowed

also:
* do not open calendar when option is not checked in QGIS project
* define inputMask whenever easily possible

fix #9
fix #215
fix #371

@3nids 3nids force-pushed the time_v2 branch from 222960e to d803b0c Jan 28, 2019

3nids added some commits Jan 28, 2019

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Jan 28, 2019

Here is a new version that should work.


There is one case I fail to solve easily or properly, when using a shapefile, which has date type only.

QGIS passes a QDate object which is translated to a JS Date object, which has no distinction for dates only.
Hence, the new date gets a time zone.
So a date coming from the shapefile as 2001-01-01 will become 2000-12-31 19:00:00 -05 (in the carribeans time zone ;) ) in QML/JS.
And when formatting this to a date only, this is shown as 2000-12-31.

This issue doesn't occur with gpkg as field has a date/time type.
I am not really sure how and where to handle this.

ping @m-kuhn


@saemiw could you give another try at this one?

3nids added some commits Jan 28, 2019

temporarily disable test and give name to jobs
waiting for PR qgis/QGIS#9011 then rebuild of new image to have test passing
@qfield-fairy

This comment has been minimized.

Copy link
Collaborator

qfield-fairy commented Jan 28, 2019

Uploaded test apks for armv7 and x86

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Jan 28, 2019

@saemiw ready for testing!

@saemiw

This comment has been minimized.

Copy link

saemiw commented Jan 29, 2019

@3nids now we're talking! Everything works as expected. Great. I'll ask my work collegue to test it as well, as she was primiraly asking for that feature. But don't expect major problems. Cool.

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Jan 29, 2019

@saemiw thanks a lot for testing!

- echo "travis_fold:start:docker-pull"
- docker pull opengisch/qfield-sdk:${QFIELD_SDK_VERSION}
- echo "travis_fold:end:docker-pull"
- docker run -v $(pwd):/usr/src/qfield -e "BUILD_FOLDER=build-${ARCH}" -e "ARCH=${ARCH}" -e "STOREPASS=${STOREPASS}" -e "KEYNAME=${KEYNAME}" -e "KEYPASS=${KEYPASS}" -e "VERSION=${TRAVIS_TAG}" opengisch/qfield-sdk:${QFIELD_SDK_VERSION} /usr/src/qfield/scripts/docker-build.sh
- ./scripts/upload-artifacts.sh

jobs:
allow_failures:
- name: testing

This comment has been minimized.

@m-kuhn

m-kuhn Jan 29, 2019

Member

what's this for?

This comment has been minimized.

@3nids

3nids Jan 29, 2019

Author Member

test is currently failing because of a missing declare metatype in QGIS core (PR merged, waiting for new image to be built)

@3nids 3nids merged commit db73319 into master Jan 29, 2019

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@3nids 3nids deleted the time_v2 branch Jan 29, 2019

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Jan 29, 2019

merging this as is, we will try to fix the shapefile issue in a later step.

@saemiw

This comment has been minimized.

Copy link

saemiw commented Jan 29, 2019

my collegue suggest also one more change (maybe also in another step/together with the shapefile issue-fix): When you enter non-sense like 30:00:00 in a hh:mm:ss-field QField does show these numbers in grey (instead of black) and then doesn't save them when you save the feature. I would be better when an non-sense entry like 30:00:00 would immediately disappear after pushing the o.k. button instead of showing the wrong entry in grey. Like this it would be more obvious to the fieldworker that these data will not be saved.
Is this doable? Its still good to incorporate the current solution but maybe improve it later.

@3nids

This comment has been minimized.

Copy link
Member Author

3nids commented Jan 29, 2019

fix or shapefiles in #458

@saemiw good idea, I'll see if I can do it in a second step...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment