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

update HTML data provider metadata #5700

Merged
merged 3 commits into from
Dec 13, 2017
Merged

Conversation

Gustry
Copy link
Contributor

@Gustry Gustry commented Nov 22, 2017

Description

Bringing back the HTML metadata from the raster provider. (Pixel size, dimension, Origin etc ...)

No more the old glossy. I can still improve the CSS later

This PR only updates the old HTML

Tickets:

Checklist

Reviewing is a process done by project maintainers, mostly on a volunteer basis. We try to keep the overhead as small as possible and appreciate if you help us to do so by completing the following items. Feel free to ask in a comment if you have troubles with any of them.

  • Commit messages are descriptive and explain the rationale for changes
  • Commits which fix bugs include fixes #11111 in the commit message next to the description
  • Commits which add new features are tagged with [FEATURE] in the commit message
  • Commits which change the UI or existing user workflows are tagged with [needs-docs] in the commit message and containt sufficient information in the commit message to be documented
  • I have read the QGIS Coding Standards and this PR complies with them
  • This PR passes all existing unit tests (test results will be reported by travis-ci after opening this PR)
  • New unit tests have been added for core changes
  • I have run the scripts/prepare-commit.sh script before each commit

@@ -510,8 +510,7 @@ class CORE_EXPORT QgsRasterDataProvider : public QgsDataProvider, public QgsRast
//! \note not available in Python bindings
static QStringList cStringList2Q_( char **stringList ) SIP_SKIP;

static QString makeTableCell( const QString &value );
static QString makeTableCells( const QStringList &values );
static QString makeHtmlBulletList( const QStringList &values );
Copy link
Member

Choose a reason for hiding this comment

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

Can this methods be private? Possibly even a generic QgsHtmlUtils class would be nice to create the same kind of lists throughout QGIS.
Mainly it shouldn't be exposed by the raster data api.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, better! It's fixed.

Copy link
Member

Choose a reason for hiding this comment

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

A lot better, thanks!

@Gustry Gustry force-pushed the raster_provider branch 2 times, most recently from 310fbbd to 616da5b Compare November 23, 2017 08:31
{
QString s( QStringLiteral( "<ul>" ) );

for ( QStringList::const_iterator i = values.begin();
Copy link
Member

Choose a reason for hiding this comment

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

How about a for ( const QString &value : values ) loop?

i != values.end();
++i )
{
s += QLatin1String( "<li>" ) + *i + QLatin1String( "</li>" );
Copy link
Member

Choose a reason for hiding this comment

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

If in doubt, use a template based approach QStringLiteral( "<li>%1</li>" ).arg( value );

* \ingroup coregit st
* \class QgsHtmlUtils
* \brief Class for HTML utilities.
* \since QGIS 3.0
Copy link
Member

Choose a reason for hiding this comment

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

Is this targetting 3.0 or 3.2?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It's fixing bug https://issues.qgis.org/issues/17504 so 3.0, Is-it not possible?

BTW, I will fix the + * \ingroup coregit st visible here L24

Copy link
Member

Choose a reason for hiding this comment

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

Wasn't sure, if it fixes a bug, 3.0 is the way to go 👍

@m-kuhn m-kuhn added the Bugfix label Nov 23, 2017
@Gustry Gustry force-pushed the raster_provider branch 3 times, most recently from 88b15cf to 06ea959 Compare November 23, 2017 09:48
@Gustry
Copy link
Contributor Author

Gustry commented Nov 23, 2017

Failing test seems unrelated to the PR https://dash.orfeo-toolbox.org/viewTest.php?onlyfailed&buildid=299224

@Gustry
Copy link
Contributor Author

Gustry commented Nov 28, 2017

Travis is happy again. Anything to else to fix?

@Gustry
Copy link
Contributor Author

Gustry commented Dec 12, 2017

Can we merge this PR? Thanks

@timlinux timlinux merged commit 5c28eca into qgis:master Dec 13, 2017
@timlinux
Copy link
Member

Thanks @Gustry !

@Gustry Gustry deleted the raster_provider branch January 8, 2018 05:12
mj10777 pushed a commit to mj10777/QGIS that referenced this pull request Feb 3, 2018
* update HTML data provider metadata for grass, gdal and ams

* update HTML data provider metadata for WMS and WCS

* move HTML bullet list to QgsHtmlUtils
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