art-institute-of-chicago / browser-extension

Browser extension to view a random artwork from our collection in a new browser tab
GNU Affero General Public License v3.0
14 stars 6 forks source link

Add settings to Chrome extension #19

Open IllyaMoskvin opened 5 years ago

yupitsleen commented 5 years ago

Hi @IllyaMoskvin, I'd like to help with this project (this issue and any others)--are you accepting contributions? Thanks!

nikhiltri commented 5 years ago

Yes! You’re welcome to submit a pull request, or talk through some ideas on here. Thanks for any contributions you make!

dylanirlbeck commented 4 years ago

@nikhiltri @IllyaMoskvin I'd love to help add a filter for the extension, but I'd first like to know what you all have in mind (and what's possible from the API). At least personally, I was thinking you could have the following settings:

I really would love to contribute, but I just don't know where to start/what would be merged in. Please let me know your thoughts!

IllyaMoskvin commented 4 years ago

@dylanirlbeck Thank you for the interest! Contributions are very welcome. I'll attempt to capture some of the discussions we've had around the office, for what we were planning for this feature.

We'd like to add a settings panel for the plugin, where users could select filters for the random query. A good place to start would be to mirror some of the functionality that's present in the "Filters" sidebar of our website's collection page:

https://www.artic.edu/collection

In terms of the design for the settings panel, I wouldn't worry too much about making it on-brand at this stage. I'd say using reset.css and keeping the code clean would be enough to start. We can do a design pass as part of the merge.

In terms of showing active filters on the frontend, I created #47 to track that effort. Overall, showing the filters on the frontend is not a blocker to getting the filter functionality working, but I think it's a good next step.

In terms of which filters to focus on for the initial push, we thought these ones might be nice:

As mentioned in #45, we should hold off on implementing that issue for the moment, since it would require coordinating with content folks at the museum to add new data to artwork records. We can do so once we have a filter infrastructure in place.

Let's put aside styles and departments for a moment, since they are more complicated, and just talk about how we would implement the date range filter. First, we'll need to have some sort of input to get desired date_start and date_end values. This could be a pair of text inputs, a range slider like the one on our collections, or whatever. We just need to get two integers for the years.

To get artworks that fall within a certain date range, we'll need to use an Elasticsearch range query:

https://www.elastic.co/guide/en/elasticsearch/reference/6.0/query-dsl-range-query.html

For example, this returns artworks that were were created between 1812 and 1815, inclusive:

curl --location --request POST 'https://api.artic.edu/api/v1/search' \
--header 'Content-Type: application/json' \
--data-raw '{
  "resources": "artworks",
  "query": {
    "bool": {
      "filter": [
        {
          "range": {
            "date_start": {
              "gte": 1812
            }
          }
        },
        {
          "range": {
            "date_end": {
              "lte": 1815
            }
          }
        }
      ]
    }
  }
}'

Above, we are actually issuing two range queries that are combined within a single filter query. When you combine multiple query clauses like that in a filter context, they are additive. You can read more here:

https://www.elastic.co/guide/en/elasticsearch/reference/6.0/query-dsl-bool-query.html

In order to integrate these queries into our application, they need to be added here:

https://github.com/art-institute-of-chicago/browser-extension/blob/bc41582/script.js#L238-L259

Styles and departments are what we refer to internally as "categorical" filters. They are more complicated. On the frontend, it's still pretty simple: we'll just end up adding a couple more filter clauses, using the Elasticsearch term query:

https://www.elastic.co/guide/en/elasticsearch/reference/6.0/query-dsl-term-query.html

For example, let's say you want to get all artworks with the style "Impressionism" that are in the department "American Art". Your query on the frontend might look like this:

curl --location --request POST 'https://api.artic.edu/api/v1/search' \
--header 'Content-Type: application/json' \
--data-raw '{
    "resources": "artworks",
    "fields": [
        "id",
        "title",
        "style_ids",
        "style_titles",
        "department_id",
        "department_title"
    ],
    "query": {
        "bool": {
            "filter": [
                {
                    "term": {
                        "style_titles.keyword": "Impressionism"
                    }
                },
                {
                    "term": {
                        "department_title.keyword": "American Art"
                    }
                }
            ]
        }
    }
}'

The tricky part is figuring out how to let the user choose the values "Impressionism" and "American Art" in the first place. In the settings panel, in order to get the filter options (facets?) for styles and departments, you'll need to perform Elasticsearch terms aggregations:

https://www.elastic.co/guide/en/elasticsearch/reference/6.0/search-aggregations-bucket-terms-aggregation.html

Here's the query we use to get the options for departments:

curl --location --request POST 'https://api.artic.edu/api/v1/search' \
--header 'Content-Type: application/json' \
--data-raw '{
    "resources": "artworks",
    "size": 0,
    "aggregations": {
        "departments": {
            "terms": {
                "field": "department_title.keyword"
            }
        }
    }
}'

Here's the query to get the options for styles:

curl --location --request POST 'https://api.artic.edu/api/v1/search' \
--header 'Content-Type: application/json' \
--data-raw '{
    "resources": "artworks",
    "size": 0,
    "aggregations": {
        "styles": {
            "terms": {
                "field": "style_titles.keyword"
            }
        }
    }
}'

Additionally, we'll want to offer small search boxes for each categorical filter (i.e. styles and departments) in this example, so that the user can search through the available facets.

The way to do this is a little tricky. For example, if the user types "imp" into the search box for styles, here's what our aggregation query will look like:

curl --location --request POST 'https://api.artic.edu/api/v1/search' \
--header 'Content-Type: application/json' \
--data-raw '{
    "resources": "artworks",
    "size": 0,
    "aggregations": {
        "styles": {
            "terms": {
                "field": "style_titles.keyword",
                "include": ".*[Ii]mp.*"
            }
        }
    }
}'

So we take the value of that search box, slice the first character i, make it both capitalized and lowercased in brackets [Ii], concat it with the rest of the search query [Ii]mp, and both prefix and suffix it with wildcards .*.

No worries if this seems like a lot. Elasticsearch is awesome, but it's a lot to take in if you've never dealt with it before. If this all makes sense⁠—great! We really need to put together some tutorials like this on our API documentation.

That should be just about all the institutional knowledge that would be involved in making this feature. The rest, I'm guessing, should be pretty standard JavaScript and browser extension stuff that could be found elsewhere. I've never had to make a settings page for an extension, so that's an unknown to me. I'd also recommend taking a look at how we are storing data between tab views. I don't know the mechanism by which settings will be saved and communicated to the frontend. That'll be interesting to see!

I hope this helps you and anyone else who is thinking about contributing to this project. As always, we are happy to answer questions as they may arise. Thanks again for your interest!

dylanirlbeck commented 4 years ago

@IllyaMoskvin I really appreciate your thorough response! Elasticsearch is not a tool I'm familiar with, but it seems fairly straightforward; plus, you've already given me concrete directions on how to use it!

My plan is to implement each of the three filters separately, starting with the date filter. From a design standpoint, I'll try and model the current UI on the official collections page (as you mentioned), but I'll definitely need some guidance from your designers before doing the final merge.

Speaking of which, should I be making each of these pull requests against master, or is there a more ideal head branch? Like I said, I'm planning on going one pull request at a time to break up the work into more sizeable/reviewable chunks.

Thanks again! I'll thread additional questions below as they come to me.

IllyaMoskvin commented 4 years ago

@dylanirlbeck Glad to hear it! That approach sounds good to me. Doing them one at a time works for us. Whenever you're ready with the date filter, go ahead and make a PR, and we'll ask our designers for feedback. I imagine the first settings-related PR might take a little longer for us to merge than subsequent ones, as we'll need time to get that initial feedback, but it should go faster on our end after we have a model to follow.

If you don't mind, please check the "Allow edits from maintainers" option when making your PR. This will allow us to push commits to your branch prior to making the merge. It's possible that our designers might prefer to do a quick live-coding session with us, where we make adjustments on the fly, rather than providing mockups or instructions. In that case, it might be easier for us to just push that code to your branch, rather than asking you to redo that work. To be determined!

Yes, please make PRs against master! Thanks for that question: I didn't realize how many old branches there were in this project. All the outdated ones should be gone now.

Thank you, and good luck!

dylanirlbeck commented 4 years ago

@IllyaMoskvin Quick design question -- I imagine that the ideal filter settings design would be the same as what's on the collections page:

image

This might be a stretch, but is there any way it's possible to re-use the existing code/CSS for that filter? I'm not sure how feasible it is, but if anything it would ensure a consistent experience across all of the Art Institute's projects (in addition to making my life easier 😄 ). Thanks in advance!

EDIT: If you prefer to talk via Slack, I'm also in the Chi Hack Night workspace -- feel free to message me there since that might be easier to share code and such.

IllyaMoskvin commented 4 years ago

@dylanirlbeck Ah, that's a tough question. We've run into this issue before, i.e. wanting to reuse the website's stylesheets elsewhere as a sort of templating engine, similar to Bootstrap.

Without getting too deep into it here, I don't think that we can use that code directly. Our website's frontend was architected in a modular manner (following BEM and atomic design), but it wasn't built in a way where it could be reused like a framework for new projects. I suspect that attempting to use the website templates in this context may result in us fighting the styles more than them being helpful. Ideally, we'd like to extract some of that code from the website into a shared library, but we aren't there yet. Maybe one day!

That said, maybe these filters are a good use-case to start thinking about how to extract some of that code, e.g. the parts having to do with forms. But also, I don't want that to be a blocker for your work, or the work of other external contributors. Given that our website is currently closed-source, it'll be up to us internal developers to work on that extraction, and we don't have time scoped for that sort of work in the near future.

Thanks for the heads-up about Slack! I'll reach out to start a conversation. We should still aim to document major points of discussion here for public reference, but I agree that this maybe isn't the best medium for back-and-forth code questions.