AtlasOfLivingAustralia / avh-hub

Australian Virtual Herbarium
https://avh.ala.org.au
Mozilla Public License 2.0
4 stars 2 forks source link

Add data quality filter functionality to AVH #120

Closed nickdos closed 8 months ago

nickdos commented 3 years ago

DQ filters were added to ALA Biocache in early 2021, see screenshot:

image

For AVH, the suggestion (from NK) is to have a dedicated "AVH" data profile, that HISCOM will advise on. The other change compared to ALA would be to have the DQ filters disabled by default (so opt-in, not opt-out like ALA) and for the filters to be hidden by default too. Similar to this:

image

nickdos commented 3 years ago

First version now up on test - https://avh-test.ala.org.au/occurrences/search?taxa=acacia+dealbata

PR: https://github.com/AtlasOfLivingAustralia/biocache-hubs/pull/473

nickdos commented 3 years ago

@javier-molina would you mind QA testing this issue, pls?

javier-molina commented 3 years ago

I'm on it

javier-molina commented 3 years ago

Ok these are my findings so far, prior to my first meeting:

1. PRESUMED_SWAPPED_COORDINATE page missing, I haven't checked whether it is a documentation or config issue yet, I won't make any assumptions Screen Shot 2021-08-27 at 9 38 53 am

2. These below are incomplete URLs, I seem to remember this is an existing limitation (bug) of current implementation when filters don't return records for a particular assertion/field. Probably @brucehyslop can refresh our memory but I'm inclined to think there is nothing to do here until the underlying implementation is fixed.

Screen Shot 2021-08-27 at 9 35 58 am

3. Styling issues, essentially text link colors 3.1 Here below the red text MUST be black as it is not clickable. Screen Shot 2021-08-27 at 10 15 18 am

3.2 Text links must be green Screen Shot 2021-08-27 at 9 31 19 am

3.3 Text links must be green Expanded link must be black as it is not a link.

I would prefer that disabled/unchecked filters had black text "Unchecked" for consistency but that is beyond this ticket. Screen Shot 2021-08-27 at 11 52 11 am

4.4 See above, Exclude records pre 1700 does not have a link to Expand and edit filters Works fine in incognito mode though

User error, see last comment on 3.3

nielsklazenga commented 3 years ago

I was looking at suggesting a fix for the first styling issue, but the red colour is added using an inline style, so cannot be changed using CSS:

<span style="color: #C44D34">
  <span class="tooltips" data-hasqtip="65" oldtitle="Exclude records with a spatially suspect (geospatial kosher) flag." title="" aria-describedby="qtip-65">Exclude spatially suspect records</span>&nbsp;
  <a href="#DQCategoryDetails" class="DQCategoryDetailsLink" data-profilename="ALA General" data-dqcategoryname="Exclude spatially suspect records" data-categorylabel="spatially-suspect" data-fq="-spatiallyValid:&quot;false&quot;" data-description="[Exclude all records where spatial validity is &quot;false&quot;]" data-translation="{&quot;false&quot;:&quot;Spatially suspect&quot;}" data-disabled="false" data-toggle="modal" role="button"><i class="fa fa-info-circle tooltips" data-hasqtip="66" oldtitle="Click for more information and actions" title=""></i></a>&nbsp;
  <span class="facet-count">
    <i class="fa fa-circle-o-notch fa-spin exclude-loader" style="display: none;"></i>
    <span style="" class="exclude-count-facet" data-category="spatially-suspect">-(15,240)</span>
  </span>
</span>

The second and third styling issues can be easily fixed by adding the following CSS:

#DQCategoryDetails .btn-link, #DQManageFilters .btn-link {
    color: #006600;
}

#DQCategoryDetails #excluded .exclude-count-label {
    color: inherit;
    font-style: normal;
}

BTW, when this was tested for the Biocache, did you ever get the comment that, in the 'Filter selection' modal, the checkbox and the 'Expand and edit filters' link should be in separate columns? Now it looks like the latter is the label for the former, while clicking on the link (which really is a button that is made to look like a link) accomplishes something entirely different than clicking on the checkbox.

javier-molina commented 3 years ago

@alexhuang091, cc @nickdos Back to you, there are mostly styling issues to address, functionality wise looks good.

I had an issue originally in the RH filter panel but I could not reproduce it again, hence nothing to see there.

javier-molina commented 3 years ago

Inline styles are evil.

I was looking at suggesting a fix for the first styling issue, but the red colour is added using an inline style, so cannot be changed using CSS:

<span style="color: #C44D34">
  <span class="tooltips" data-hasqtip="65" oldtitle="Exclude records with a spatially suspect (geospatial kosher) flag." title="" aria-describedby="qtip-65">Exclude spatially suspect records</span>&nbsp;
...
alexhuang091 commented 3 years ago

2 Hi @javier-molina you are correct on point2. If there's no wiki for a specific value, the wiki of the key (in this case assertion ) will be displayed

nickdos commented 3 years ago

Agree inline CSS should be removed, as it breaks other hubs display - including any LA portals that might use DQ filters in the future.

nielsklazenga commented 3 years ago

I found another one:

image

Can be resolved like this:

div#DQPrefSettings .text-info {
    color: inherit;
    font-weight: normal;
    line-height: 1.42857143;
}

There is actually a bigger issue here, which is that the modal is sitting inside the <div class="activeFilters" />, rather than right below the <body />, or at least the highest-order element of the DQ filters. That's why I had to fix the line height, which it should inherit from the body, but now inherits from the .activeFilters class. I am not suggesting that that should be fixed right now.

hxuanyu commented 3 years ago

The

Learn More is a config issue. The data_quality_learnmore_link is not defined in avh-test

1

hxuanyu commented 3 years ago

View excluded records link sometimes is wrong. It's also an issue on prod

3 2

hxuanyu commented 3 years ago

the DQ category detail launched from the left side panel is not complete

4

alexhuang091 commented 3 years ago

Hi @javier-molina, all issues fixed. Could please re-test it. Thanks

javier-molina commented 3 years ago

I'm on it after my two day exile.

javier-molina commented 3 years ago

Works for me.

BTW good call in making the text clickable in the DQ Filters in the LHS

nielsklazenga commented 3 years ago

I have one more minor niggle:

image

The number of records excluded is not a link, so should not look like a link. Also, I thought we were avoiding italics. Bold (but not green) would be better.

nielsklazenga commented 3 years ago

Sorry, accident.

nickdos commented 3 years ago

I agree @nielsklazenga and thought the same thing when i first saw it. But I had already argued for like a dozen changes before this, so decided to pick my battles and let it slip.

I think it should be bold text but NOT italics or coloured text. Its got a CSS class so is a one-line change. Will try now.

nickdos commented 3 years ago

Deployed - try it again pls @nielsklazenga.

Edit: I did notice another bug - if you click on the (i) icon for a DQ set with zero records removed, the display shows:

image

~I think this will have to wait for next release, as we've already done biocach-hubs release. Prod shows same issue, so not a regression bug~. Prod ala-hub does show 0 records there, so it looks like a regression bug and needs fixing...

Created issue: https://github.com/AtlasOfLivingAustralia/biocache-hubs/issues/495

nielsklazenga commented 3 years ago

@nickdos that number looks good now.