-
Notifications
You must be signed in to change notification settings - Fork 49
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
☄️ Valkyrize Hyku EPIC #2129
☄️ Valkyrize Hyku EPIC #2129
Conversation
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var allColorInputs = $("input[name*='color']") | ||
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() | ||
|
||
var allColorInputs = $("input[name*='color']") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
}) | ||
|
||
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var input = $("input[name='admin_appearance["+ defaultTarget +"]']") | ||
|
||
input.val(input.data('default-value')) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var defaultTarget = $(e.target).data('default-target') | ||
var input = $("input[name='admin_appearance["+ defaultTarget +"]']") | ||
|
||
input.val(input.data('default-value')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
e.preventDefault() | ||
|
||
var defaultTarget = $(e.target).data('default-target') | ||
var input = $("input[name='admin_appearance["+ defaultTarget +"]']") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
$('div.defaultable-colors a.restore-default-color').click(function(e) { | ||
e.preventDefault() | ||
|
||
var defaultTarget = $(e.target).data('default-target') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
@@ -0,0 +1,20 @@ | |||
$(document).on('turbolinks:load', function() { | |||
$('div.defaultable-colors a.restore-default-color').click(function(e) { | |||
e.preventDefault() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var allColorInputs = $("input[name*='color']") | ||
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() | ||
|
||
var allColorInputs = $("input[name*='color']") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
}) | ||
|
||
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var input = $("input[name='admin_appearance["+ defaultTarget +"]']") | ||
|
||
input.val(input.data('default-value')) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var defaultTarget = $(e.target).data('default-target') | ||
var input = $("input[name='admin_appearance["+ defaultTarget +"]']") | ||
|
||
input.val(input.data('default-value')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
e.preventDefault() | ||
|
||
var defaultTarget = $(e.target).data('default-target') | ||
var input = $("input[name='admin_appearance["+ defaultTarget +"]']") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
$('div.defaultable-colors a.restore-default-color').click(function(e) { | ||
e.preventDefault() | ||
|
||
var defaultTarget = $(e.target).data('default-target') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
@@ -0,0 +1,20 @@ | |||
$(document).on('turbolinks:load', function() { | |||
$('div.defaultable-colors a.restore-default-color').click(function(e) { | |||
e.preventDefault() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var allColorInputs = $("input[name*='color']") | ||
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() | ||
|
||
var allColorInputs = $("input[name*='color']") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
}) | ||
|
||
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
if (domDependenciesMet()) { | ||
// Grab the data from the ul div | ||
var series_data = new Array(); | ||
var pointer_lookup = new Array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array literal notation [] is preferable.
//flot loaded? And canvas element supported. | ||
if (domDependenciesMet()) { | ||
// Grab the data from the ul div | ||
var series_data = new Array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array literal notation [] is preferable.
}; | ||
|
||
global.BlacklightRangeLimit = BlacklightRangeLimit; | ||
}(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected an assignment or function call and instead saw an expression.
max = BlacklightRangeLimit.parseNum($(range_element).find(".max").first().text()); | ||
} | ||
|
||
return [min, max] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
|
||
|
||
|
||
var min = max = BlacklightRangeLimit.parseNum(current_limit.find(".single").text()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
You might be leaking a variable (max) here.
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) | ||
}) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
var allColorInputs = $("input[name*='color']") | ||
|
||
allColorInputs.each(function() { | ||
$(this).val($(this).data('default-value')) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() | ||
|
||
var allColorInputs = $("input[name*='color']") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
}) | ||
|
||
$('.panel-footer a.restore-all-default-colors').click(function(e) { | ||
e.preventDefault() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
if (domDependenciesMet()) { | ||
// Grab the data from the ul div | ||
var series_data = new Array(); | ||
var pointer_lookup = new Array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array literal notation [] is preferable.
//flot loaded? And canvas element supported. | ||
if (domDependenciesMet()) { | ||
// Grab the data from the ul div | ||
var series_data = new Array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array literal notation [] is preferable.
}; | ||
|
||
global.BlacklightRangeLimit = BlacklightRangeLimit; | ||
}(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected an assignment or function call and instead saw an expression.
max = BlacklightRangeLimit.parseNum($(range_element).find(".max").first().text()); | ||
} | ||
|
||
return [min, max] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
|
||
|
||
|
||
var min = max = BlacklightRangeLimit.parseNum(current_limit.find(".single").text()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
You might be leaking a variable (max) here.
// Grab the data from the ul div | ||
var series_data = new Array(); | ||
var pointer_lookup = new Array(); | ||
var x_ticks = new Array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array literal notation [] is preferable.
if (domDependenciesMet()) { | ||
// Grab the data from the ul div | ||
var series_data = new Array(); | ||
var pointer_lookup = new Array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array literal notation [] is preferable.
//flot loaded? And canvas element supported. | ||
if (domDependenciesMet()) { | ||
// Grab the data from the ul div | ||
var series_data = new Array(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The array literal notation [] is preferable.
}; | ||
|
||
global.BlacklightRangeLimit = BlacklightRangeLimit; | ||
}(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Expected an assignment or function call and instead saw an expression.
max = BlacklightRangeLimit.parseNum($(range_element).find(".max").first().text()); | ||
} | ||
|
||
return [min, max] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
} | ||
|
||
begin_el.val(min); | ||
end_el.val(max); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'end_el' used out of scope.
}); | ||
} | ||
|
||
begin_el.val(min); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'begin_el' used out of scope.
|
||
// Slider change should update text input values. | ||
var parent = $(this).parent(); | ||
var form = $(parent).closest(".limit_content").find("form.range_limit"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'form' is already defined.
|
||
if (plot && slider_el) { | ||
slider_el.width(plot.width()); | ||
slider_el.css("display", "block") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Missing semicolon.
if (isInt(min) && isInt(max)) { | ||
$(this).contents().wrapAll('<div style="display:none" />'); | ||
|
||
var range_element = $(this); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
'range_element' is already defined.
} | ||
|
||
.hyc-banner { | ||
padding: 15px; | ||
.hyc-bugs .hyc-created-by, .hyc-bugs .hyc-last-updated, .hyc-title { | ||
.hyc-bugs .hyc-created-by, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rule declaration should be preceded by an empty line
@@ -22,6 +22,6 @@ | |||
|
|||
// for catalog search result snippets | |||
.highlight { | |||
background: yellow; | |||
background: #ffff00; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Line should be indented 2 spaces, but was indented 4 spaces
Having this file was causing double rendering of bulkrax links in the sidebar. Issues: - #1850 - scientist-softserv/hykuup_knapsack#111
Update bulkrax to point to hyrax-4-valkyrie-support
This commit will adjust for the changes introduced in this commit: samvera/hyrax@8fc0894
bulrax_identifier is not a required property. Each client should set their desired source_identifier in their application. Issue: - scientist-softserv/hykuup_knapsack#136
This commit will add a decorator to check the `human_readable_type` of the given object is a Valkyrie migration object (which by convention ends with 'resource') and adjust the generated link accordingly. For example, a GenericWorkResource should generate links like `/concern/generic_work_resources/...` instead of `/concern/generic_works/...`. The models were updated to include the Hyrax::NestedWorks so the `Attach Child` button would populate the appropriate work types. Lastly, the `Gemfile.lock` was updated to include the latest version of Hyrax `double_combo` branch.
For us to leverage the useful shared specs of Hyrax, we need a test adapter.
…ifier-to-bulkrax I903 remove bulkrax_identifier dependency
Using 'human_readable_type' was going to be flemsy. Instead we are opting for adding the `valkyrie_bsi` to the index and asking that.
The featured works and collections had some funky css problems where it made sorting them impossible. This commit will override some Hyrax scss and fix some css classes on partials to make the featured works/collections a better user experience.
background: #ddd; | ||
background: -webkit-linear-gradient(top, #ddd 0%, #bbb 100%); | ||
background: -moz-linear-gradient(top, #ddd 0%, #bbb 100%); | ||
background: linear-gradient(top, #ddd 0%, #bbb 100%); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Property background
already defined on line 479
.dd3-handle { | ||
position: absolute; margin: 0; left: 0; top: 0; cursor: pointer; width: 30px; | ||
text-indent: 100px; | ||
white-space: nowrap; overflow: hidden; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Property 'overflow' should be placed on own line
@@ -467,6 +471,24 @@ body.public-facing { | |||
padding: 0; | |||
} | |||
|
|||
.dd3-handle { | |||
position: absolute; margin: 0; left: 0; top: 0; cursor: pointer; width: 30px; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Properties should be ordered background, border, border-bottom-right-radius, border-top-right-radius, cursor, left, margin, overflow, position, text-indent, top, white-space, width, z-index
Property 'cursor' should be placed on own line
Property 'left' should be placed on own line
Property 'margin' should be placed on own line
Property 'top' should be placed on own line
Property 'width' should be placed on own line
@@ -467,6 +471,24 @@ body.public-facing { | |||
padding: 0; | |||
} | |||
|
|||
.dd3-handle { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rule set contains (17/10) properties
Fixes bug deleting works & file sets. Refs scientist-softserv/hykuup_knapsack#187
config/application.rb
Outdated
@@ -178,9 +205,12 @@ def self.path_for(relative_path) | |||
end | |||
end | |||
|
|||
config.to_prepare do | |||
DerivativeRodeo::Generators::HocrGenerator.additional_tessearct_options = nil | |||
config.before_initialize do |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I am a bit confused why we declare the models here as well as in hyrax.rb. Which one ultimately controls the behavior? It might be helpful to have it documented why we declare them twice.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This particular commit was me attempting to squash flakey specs. I don't know if they are necessary. And I'll remove and see.
In the `double_combo` branch, we replaced the need for delegation with method_missing. Thus we can remove the delegate declarations and instead rely on method missing behavior.
Addresses all reindex jobs to ensure that they will work for either valkyrie or active fedora objects. Ensures that works and collections are appropriately reindexed as default work and collection images are added OR removed (previously, reindexing didn't occur with removal of default images, resulting in works displaying with a missing image.)
…into i35-valkyrize-hyku
…into i35-valkyrize-hyku
Since we changed the SolrEndpoint, this spec was failing. This change should fix it.
This commit will add storage/files as a persisted volume for rancher set ups. Hyrax uses this directory to store the binaries.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only the tiniest of questions and comments on this. really excellent work here
@@ -100,6 +100,9 @@ GIT | |||
specs: | |||
hyku_knapsack (0.0.1) | |||
rails (>= 5.2.0) | |||
sentry-rails |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
how did sentry become a hyku dependency?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should it not be? We added sentry for hyku up knapsack and knapsack prime, but now that you question it I wonder if we should've added it to bundler.d instead (within each repo)?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TODO: rip it out of hyku and put it in bundler.d of the respective repo.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This appears to have come about because we added sentry (et al) to the gem spec of knapsack; and when we bundled along came sentry. This is definitely a surprise as we think about traditional directional dependencies; that is we think of Knapsack as laying on top of Hyku; but in reality it's a much more convoluted relationship.
pdf_splitter_service: IiifPrint::TenantConfig::PdfSplitter | ||
) | ||
end | ||
include IiifPrint.model_configuration( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
does this mean we no longer allow IiifPrint on and off?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes…except that each tenant can disable it via a feature.
@@ -26,6 +26,8 @@ task index_collections: :environment do | |||
end | |||
end | |||
|
|||
# TODO: handle valkyrie reindex once admin sets have been valkyrized |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
aren't adminsets valkyrized now?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pushed a PR for this.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@@ -20,7 +20,7 @@ def find_ids_by_model(model:, ids: :all) # rubocop:disable Metrics/MethodLength | |||
model_name = ModelRegistry.lookup(model).model_name | |||
|
|||
solr_query = "_query_:\"{!raw f=has_model_ssim}#{model_name}\"" | |||
solr_response = ActiveFedora::SolrService.post(solr_query, fl: 'id', rows: @query_rows)['response'] | |||
solr_response = Hyrax::SolrService.post(solr_query, fl: 'id', rows: @query_rows)['response'] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
we made a generic service accessor for SolrService above - should we use it here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not aware of a generic service accessor. Which one were you thinking? There's a lot of Hyrax::SolrService
references.
@@ -32,6 +32,9 @@ extraVolumeMounts: &volMounts | |||
- name: uploads |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this file seems unused?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Standing branch to begin epic:
Included