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

Module 6.1, updated images and text. #5580

Merged
merged 13 commits into from Jun 8, 2020
Merged

Conversation

milechin
Copy link
Contributor

Updated the example snippets to reflect the GUI of QGIS 3.10. Updated the text accordingly and added some additional text to described other relevant items, such as looking at the Attribute Table after creating a new feature and describing how to access the Vertex Editor pane.

Goal:

Ticket(s): #

  • Backport to LTR documentation is required

Updated the example snippets to reflect the GUI of QGIS 3.10.  Updated the text accordingly and added some additional text to described other relevant items, such as looking at the Attribute Table after creating a new feature and describing how to access the Vertex Editor pane.
Copy link
Contributor

@havatv havatv left a comment

Choose a reason for hiding this comment

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

Thanks! I have some suggestions. And I guess that @DelazJ would like you to use numbering instead of plain bullets when describing procedures in a stepwise way.

Co-authored-by: Håvard Tveite <havard.tveite@nmbu.no>
@DelazJ
Copy link
Collaborator

DelazJ commented May 30, 2020

And I guess that @DelazJ would like you to use numbering instead of plain bullets when describing procedures in a stepwise way

😃 and I'd have added that we should move away from :kbd: being used for things other than keyboard shortcuts.

Copy link
Collaborator

@DelazJ DelazJ left a comment

Choose a reason for hiding this comment

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

@milechin Thanks for the update.
I only reviewed the diff as exposed by github. I have some suggestions and questions.
And moving to numbered list would be nice 👍

@@ -92,69 +95,71 @@ or aerial photography.
For our example, you'll be using the digitizing approach. Sample raster datasets
are provided, so you'll need to import them as necessary.

* Click on the :guilabel:`Add Raster Layer` button: |addRasterLayer|

* Click on Data Source Manager button |dataSourceManager| .
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we better use this type of instructions style

Suggested change
* Click on Data Source Manager button |dataSourceManager| .
* Click on |dataSourceManager| :sup:`Data Source Manager` button.

sure that the correct layer is selected, otherwise you'll edit the wrong
layer!)
* Click on the :guilabel:`Toggle Editing` button: |edit|
* Click on the :kbd:'school_property' layer in the :guilabel:`Layer list` to select it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

caution: there's use of ' instead of ` here. (but there's use of kbd anyways)

Four other relevant buttons are still inactive, but will become active when we
start interacting with our new data:
- |capturePolygon|: Add a new feature.
- |vertexToolActiveLayer|: move only one part of a feature.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
- |vertexToolActiveLayer|: move only one part of a feature.
- |vertexToolActiveLayer|: move vertice(s) of a feature.

would be clearer imho


Another way to edited the feature is by manually entering the coordinates for each vertex.

* Make sure the Vertex Editor tool |vertexToolActiveLayer| is still active.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Make sure the Vertex Editor tool |vertexToolActiveLayer| is still active.
* Make sure the |vertexToolActiveLayer| :sup:`Vertex Tool` is still active.

:align: center
.. figure:: img/select_vertex.png
:align: center
.. figure:: img/moved_vertext.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

It does not hurt as long as they are inline but maybe worth fixing this typo in file name ?

Suggested change
.. figure:: img/moved_vertext.png
.. figure:: img/moved_vertex.png

* Double *left-click* in the table on the x or y coordinate you want to edit and enter the
updated value.

.. figure:: img/edit_vertext_in_vertex_editor.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

Same file name "typo"

Suggested change
.. figure:: img/edit_vertext_in_vertex_editor.png
.. figure:: img/edit_vertex_in_vertex_editor.png


To enable the remaining feature editing tools, one needs to select the feature.

* Click on the Select Rectange button |selectRectangle| in the Attributes Toolbar.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
* Click on the Select Rectange button |selectRectangle| in the Attributes Toolbar.
* Click on the |selectRectangle| :sup:`Select Features` button in the Attributes Toolbar.


.. figure:: img/single_feature_select.png
:align: center
.. figure:: img/select_vertex.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

any reason of the margin left at the top in the image?

@DelazJ
Copy link
Collaborator

DelazJ commented May 30, 2020

Ouch! looks like the version I was reviewing changed in the meantime...

Copy link
Collaborator

@DelazJ DelazJ left a comment

Choose a reason for hiding this comment

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

A quick note. there are some indentation issues (I commented few) it'd be nice to address. Thanks.
you can see them either in previewing the changes in github or the html build (if you made it)


You'll be presented with the following dialog:
.. figure:: img/create_vector_layer.png
Copy link
Collaborator

Choose a reason for hiding this comment

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

rst is picky with indentation so you should align things as long as they belong to the same level. If you check the html build you'll notice a not nice vertical bar here. Should align the "Navigate".

Suggested change
.. figure:: img/create_vector_layer.png
.. figure:: img/create_vector_layer.png


You'll be presented with the following dialog:
.. figure:: img/create_vector_layer.png
:align: center
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
:align: center
:align: center

It's important to decide which kind of dataset you want at this stage. Each
different vector layer type is "built differently" in the background, so once
you've created the layer, you can't change its type.
It's important to decide which kind of dataset you want at this stage. Each
Copy link
Collaborator

Choose a reason for hiding this comment

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

Indentation. all these should align with... euh... probably moved as information in the next step?

Copy link
Contributor

@havatv havatv left a comment

Choose a reason for hiding this comment

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

Some more suggestions and comments. Mostly indentation. Not all indentation issues have been commented, so everything should be checked for indentation mistakes.

@@ -18,58 +18,57 @@ existing dataset. Therefore, you'll need to define your own new dataset first.
You'll need to open a :guilabel:`Create Layer` dialog that will allow you
to define a new layer.

* Navigate to and click on the menu entry :menuselection:`Layer --> Create Layer --> New
Shapefile Layer`.
#. Navigate to and click on the menu entry :menuselection:`Layer --> Create Layer --> New Shapefile Layer`. You'll be presented with the following dialog.
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#. Navigate to and click on the menu entry :menuselection:`Layer --> Create Layer --> New Shapefile Layer`. You'll be presented with the following dialog.
#. Navigate to and click on the menu entry
:menuselection:`Layer --> Create Layer --> New Shapefile Layer`.
You'll be presented with the following dialog.


.. figure:: img/polygon_selected.png
:align: center
.. figure:: img/polygon_selected.png
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
.. figure:: img/polygon_selected.png
.. figure:: img/polygon_selected.png

Align with the "F" in "For Geometry Type" above.

.. figure:: img/polygon_selected.png
:align: center
.. figure:: img/polygon_selected.png
:align: center
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
:align: center
:align: center

Better avoid tabs


This has no impact on the rest of the dialog, but it will cause the correct
type of geometry to be used when the vector dataset is created.
This has no impact on the rest of the dialog, but it will cause the correct
Copy link
Contributor

Choose a reason for hiding this comment

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

Identation has to be fixed for the complete "paragraph". Three spaces.

If you can't find this button, check that the :guilabel:`Digitizing` toolbar is
enabled. There should be a check mark next to the :menuselection:`View -->
Toolbars --> Digitizing` menu entry.
If you can't find this button, check that the :guilabel:`Digitizing` toolbar is
Copy link
Contributor

Choose a reason for hiding this comment

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

Indentation...

@milechin
Copy link
Contributor Author

milechin commented Jun 3, 2020

I think I fixed the indentations, but let me know if it still needs work.

I still need to go through your other suggestions when I have time.

Best,

Dennis

Copy link
Contributor

@havatv havatv left a comment

Choose a reason for hiding this comment

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

Just suggesting.

One at a time, digitize the path and the track on the :guilabel:`routes` layer.
Try to follow the routes as accurately as possible, using points (left-click) at
any corners or turns.
#. Create a new line feature called ``routes.shp`` with attributes ``id`` and ``type``. (Use the approach above to guide you.)
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
#. Create a new line feature called ``routes.shp`` with attributes ``id`` and ``type``. (Use the approach above to guide you.)
#. Create a new ESRI Shapefile line dataset called ``routes.shp``, with attributes ``id`` and
``type`` (Use the approach above to guide you.)

@DelazJ
Copy link
Collaborator

DelazJ commented Jun 4, 2020

I think I fixed the indentations, but let me know if it still needs work

Nice! Thanks


For the next exercise, we're going to be creating new features which describe
areas. For such features, you'll need to create a polygon dataset.
For the next exercise, we're going to be creating new features which describe
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
For the next exercise, we're going to be creating new features which describe
For the next exercise, we're going to create new features which describe

@milechin
Copy link
Contributor Author

milechin commented Jun 5, 2020

I think I have incorporated all of the suggestions and corrections. Let me know if you have any other ones.

Copy link
Contributor

@havatv havatv left a comment

Choose a reason for hiding this comment

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

Looks fine to me. I have some suggestions I hope you will consider.

Co-authored-by: Håvard Tveite <havard.tveite@nmbu.no>
Copy link
Collaborator

@DelazJ DelazJ left a comment

Choose a reason for hiding this comment

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

Sorry I did not reply earlier @milechin. But you seem to get all I had in mind.
This is a pretty good update. Thanks.

@@ -92,165 +96,255 @@ or aerial photography.
For our example, you'll be using the digitizing approach. Sample raster datasets
are provided, so you'll need to import them as necessary.

* Click on the :guilabel:`Add Raster Layer` button: |addRasterLayer|
#. Click on Data Source Manager button |dataSourceManager| :sup:`Data Source Manager` button.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#. Click on Data Source Manager button |dataSourceManager| :sup:`Data Source Manager` button.
#. Click on |dataSourceManager| :sup:`Data Source Manager` button.

?

areas. For such features, you'll need to create a polygon dataset.
#. Click :guilabel:`...` for the :guilabel:`File name` field.
A save dialog will appear.
#. Navigate to the ``exercise_data`` directory.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#. Navigate to the ``exercise_data`` directory.
#. Navigate to the :file:`exercise_data` directory.

@@ -92,165 +96,255 @@ or aerial photography.
For our example, you'll be using the digitizing approach. Sample raster datasets
are provided, so you'll need to import them as necessary.

* Click on the :guilabel:`Add Raster Layer` button: |addRasterLayer|
#. Click on Data Source Manager button |dataSourceManager| :sup:`Data Source Manager` button.
#. Select :guilabel:`Raster` on the left side.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#. Select :guilabel:`Raster` on the left side.
#. Select |raster| :guilabel:`Raster` on the left side.

You can ignore this one if you want.
If you don't, don't forget the substitution

.. |raster| image:: /static/common/mIconRaster.png
   :width: 1.5em

To enter edit mode for the :guilabel:`school_property` layer:
To enter edit mode for the ``school_property`` layer:

#. Click on the ``school_property`` layer in the :guilabel:`Layer list` to select it.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#. Click on the ``school_property`` layer in the :guilabel:`Layer list` to select it.
#. Click on the ``school_property`` layer in the :guilabel:`Layers` panel to select it.

If we want to conform to the GUI label

.. figure:: img/vertex_selected.png
:align: center

#. To update the a coordinate, double left click on a cell in the table
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#. To update the a coordinate, double left click on a cell in the table
#. To update a coordinate, double left click on the cell in the table

#. One at a time, digitize the path and the track on the ``routes`` layer.
Try to follow the routes as accurately as possible, adding additional points along
corners or turns.
#. Set the ``type`` attribute value to ``path`` or ``track``.\
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
#. Set the ``type`` attribute value to ``path`` or ``track``.\
#. Set the ``type`` attribute value to ``path`` or ``track``.

Or is it on purpose?

Copy link
Contributor

@havatv havatv left a comment

Choose a reason for hiding this comment

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

Looks fine. Some lines on the longer side...

Co-authored-by: Håvard Tveite <havard.tveite@nmbu.no>
@DelazJ DelazJ merged commit 441aa09 into qgis:release_3.10 Jun 8, 2020
@DelazJ
Copy link
Collaborator

DelazJ commented Jun 8, 2020

Thanks @milechin for the hard work and your patience

DelazJ added a commit to DelazJ/QGIS-Documentation that referenced this pull request Jun 8, 2020
@milechin
Copy link
Contributor Author

milechin commented Jun 8, 2020

Sure, thank you for the feedback as well. If time allows, I will see if I can work on section 6.2 next. I am interested in having some of these sections updated so that I can use them for QGIS tutorials I periodically will be teaching.

@milechin milechin deleted the tutorialUpdate branch June 8, 2020 20:22
@DelazJ
Copy link
Collaborator

DelazJ commented Jun 9, 2020

If time allows, I will see if I can work on section 6.2 next.

Let's then hope that time allows 🙏
One thing: even if you use the 3.10 version to update the docs, better pull request the master branch; we have tools to automatically backport to 3.10 while 3.10 to master needs to be done manually (not that it takes long but... it will also put you in the docs history and avoid me stealing your work as I just realized I did - sorry I thought cherry-pick preserved authorship)

@milechin
Copy link
Contributor Author

milechin commented Jun 9, 2020

No problem. Thank you for the clarification on the best practice for updating.

@DelazJ DelazJ mentioned this pull request Jun 14, 2020
41 tasks
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