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

Upgrade to v1 breaks with applications using Bootstrap 4.6 #2522

Closed
nickcharlton opened this issue Feb 16, 2024 · 11 comments
Closed

Upgrade to v1 breaks with applications using Bootstrap 4.6 #2522

nickcharlton opened this issue Feb 16, 2024 · 11 comments

Comments

@nickcharlton
Copy link
Member

Upgrade from 0.19 to v1 beta causes the following errors on pages that use custom modifications in view templates:

ActionView::Template::Error
(undefined method `name' for ["", [
  #<Administrate::Field::Number:0x00007fe6741bc170..>,
  #<Administrate::Field::String:0x00007fe6741bc120..>,
  #<Administrate::Field::Email:0x00007fe6741bc080 ..>,
  ...
  #<Administrate::Field::BelongsTo:0x00007fe6741bb..>,
  #<Administrate::Field::String:0x00007fe6741bbd60..>,
  #<Administrate::Field::String:0x00007fe6741bbcc0..>,
  #<Administrate::Field::String:0x00007fe6741bbb80..>,
]]:Array)

Plus a bunch of weird frontend issues, like Search input becomes blue on hover, or Datepicker not working at all. Does it use Bootstrap 5 ? and if yes, if the main app already uses Bootstrap 4.6

Originally posted by @januszm in #2518 (comment)

@nickcharlton
Copy link
Member Author

@januszm, I think you're hitting a few things here, one of which was a mistake by me.

Could you try with current main and let me know what's still broken?

@januszm
Copy link
Contributor

januszm commented Feb 16, 2024

@nickcharlton thanks for your quick reply, let me clarify this as I was already able to resolve issue 1.

First, problem with template from 0.19 that was customized, and below are diffs that solved it:

- ... ) if valid_action?(:edit) && show_action?(:edit, page.resource) %>
+ ... ) if accessible_action?(page.resource, :edit) %>

(valid_action? => accessible_action?)

- <% page.attributes.each do |attribute| %>
+ <% page.attributes.each do |title, attributes| %>

this is I believe what caused the no method error name for [...] Array, (attribute vs attributes)

Search box with weird blue background on hover (see attachment)
Screenshot 2024-02-16 at 14 47 53

Datepicker actually WORKS but it uses white font color on white background which of course makes the text invisible and suggests that it isn't working. (see attachment)
Screenshot 2024-02-16 at 14 48 52

@januszm
Copy link
Contributor

januszm commented Feb 16, 2024

@nickcharlton also, the last two (visual) issues are solved on the main branch. I've just updated the gem and it looks good.

So the potential, future "Upgrade to v1" Guide should contain information about the need to re-generated the customized partials and re-apply added code. I think that's all.

@januszm
Copy link
Contributor

januszm commented Feb 16, 2024

One more request, I noticed that after the upgrade the <pre> elements don't have white-space pre-wrap set anymore. I have some dashboards where I dump jsonb data with JSON.pretty_generate. It produces <pre> tags (just a poor-mans way for supporting jsonb with administrate). In 0.19 long lines were nicely wrapped. In v1 I have to somehow inject pre { white-space: pre-wrap; }

Or, can I somehow enforce applying this class to specific field?

.preserve-whitespace {
  white-space: pre-wrap;
  word-wrap: break-word;
}

https://github.com/thoughtbot/administrate/blob/main/app/assets/stylesheets/administrate/components/_attributes.scss

@nickcharlton
Copy link
Member Author

So the potential, future "Upgrade to v1" Guide should contain information about the need to re-generated the customized partials and re-apply added code. I think that's all.

Alright, cool! I'll add a mention to explicitly call that out in the migration guide. It is something we started highlighting in the CHANGELOG, but that's always easy to miss.

@nickcharlton
Copy link
Member Author

Re: <pre>, can you tell if that's something that changed in the linting PR? If it was, I think we should undo that specific change as it wasn't intentional.

nickcharlton added a commit that referenced this issue Feb 19, 2024
This addresses some comments raised in #2522.
@januszm
Copy link
Contributor

januszm commented Feb 19, 2024

ReL <pre>, for a regular field it looks ok, both white-space and word-wrap attributes are set properly.
In my case the issue is with a custom field (data type) that I called PrettyJson (it's a type that I use to dump a typical jsonb column data).
It generates the following css class which is missing the necessary white-space attr attribute-data attribute-data--pretty-json-field

@januszm
Copy link
Contributor

januszm commented Feb 19, 2024

UPDATE: @nickcharlton please disregard my comments about <pre>. After all it was because I had a custom field with a custom template and in that template I used <pre> tag. The solution was to add a CSS class to this custom view partial.

Any chance I can tell my custom field class to use preserve-whitespace CSS class on the generated element? Just like for the Field::Text class.

@nickcharlton
Copy link
Member Author

Ah, hah! Excellent. Glad to hear you figured it out.

Is that everything you were having trouble with?

@januszm
Copy link
Contributor

januszm commented Feb 20, 2024

All good now.

I'd make this section of the migrating .md file a bit more detailed, e.g.

As with most upgrades, if you're upgrading between versions with changes to the
templates, and if you've customised them, you may need to apply recent changes.
You can see those which changed in the [CHANGELOG][].

This applies to both modified files in `app/views/admin` and `app/views/fields`
if you have your own Field classes defined. A good way is to use the same command
to generate the view as for the earlier version:
`rails generate administrate:views:ACTION MyModel`
noting the changes and re-applying the modifications

@nickcharlton
Copy link
Member Author

Oh, yes, that's a great idea. I'll do that.

I'll close this now, but please open new issues if you find problems.

nickcharlton added a commit that referenced this issue Jun 28, 2024
This addresses some comments raised in #2522.
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

No branches or pull requests

2 participants