We've experienced some caching issues with this code for years. We've
fixed some of them in commits 9979b5399 and 3645c333a, but we're still
running into other issues. In order to really cache this section, we'd
need to cache:
* Whether or not the investment should show the aside and vote/support
buttons (we could do it by caching its budget)
* Whether its budget is balloting or finished (we could do it by caching
its budget)
* Whether the current user is following the investment
* Whether the `remove_investments_supports` feature is enabled
* Whether the `community` feature is enabled
* The value of the `org_name` setting
* The value of the `twitter_handle` setting
We weren't caching all these elements, meaning that (for instance), we
didn't display the button to vote when a budget moved into the voting
phase if we had already cached this section without the button.
And chances are the list is incomplete. So, instead of trying to take
into account every single possible factor that should make us expire the
cache, we're restricting the cache so it only affects the content of the
investment. This is similar to what we do in the investments index,
where we cache the content of the investment but we don't cache the
vote/support buttons.
Since the change on commit cbbe188d6 we added a Poll.current.any?
condition to show the officing link on admin menu to officers.
That condition doesn't have much sense since Poll results only can be
added after a poll has ended, and there may be only one active poll.
The page was crashing when at least part of the content of the page had
been translated between the request showing the remote translations
button and the moment people pressed the button.
The top bar padding was different on small screens when we were in the
admin section, so we're now applying the same padding everywhere.
Note we're still applying extra padding on medium/large screens because
in the public section we display our logo image, which has some blank
space. In the admin section we're emulating this blank space with
padding; we might change it in the future if we change our logo.
Also note we're using `0.8rem` instead of `$line-height / 2`. The reason
is tricky: there's a spec testing the reorder feature with drag and drop
in the poll questions answers administration, and that test fails when
the drop space is right at the bottom of the screen, which is what
happens when we use the `$line-height / 2` padding. A proper solution
would be to remove the inaccessible drag and drop feature and use a
different method to reorder the answers.
We can use the `link` mixin. Note this mixin uses anchor-color instead
of brand-color; by default, they're both the same, so we probably meant
anchor-color here.
The `overflow: hidden` applied to the `.callout` selector made the full
width background invisible, meaning this section hasn't looked properly
on very large screens since commit 701378d02.
We were using partials to render components in order to ease the
transition of custom code from earlier versions of Consul. However,
that was back in Consul 1.4, and now these views looked a bit messy
since they sometimes rendered components and sometimes they
rendered partials.
We weren't using the `admin-top-bar` class since commit e6c1cf774, and
we can always use the `.admin .top-bar` selector if we need to. And the
`row expanded` classes basically give an element a width of 100%, which
is already the default width for block elements.
We were using the same code for the button in both the public and admin
headers, so we're removing the duplication.
Since the menu and the button must go together, and the contents of the
menu are different for different layouts, we're passing these contents
using a block.
Note the ID of the menu was `responsive-menu` in the public section but
`responsive_menu` in the admin section. Since we usually use underscores
for IDs and dashes for classes, we're keeping the one with the
underscore.
So now it uses the same interface and styles as the main layout. On
small screens, it's easier to play with the menu when the button is on
the left because the menu it opens is aligned to the left.
Note that now we can get rid of the title-bar class; we didn't use the
styles in the public area since commit dcec003d0, and we were overriding
all the Foundation styles in the admin area with the exception of the
padding, which we no longer need.
We're changing the order of the elements in the HTML so the menu button
appears next to the menu it opens, with no logo between them, which IMHO
makes sense and makes it easier to understand the layout for people
using screen readers.
A small advantage of this approach is that on very narrow screens or
Consul applications having a very long word for "Menu", the menu button
appeared on top, the logo appeared below it, and the contents of the
menu appeared below the logo. Now the logo appears on top, the menu
button appears below it, and the contents of the menu appear below the
menu button.
The menu didn't look properly on these screens since commit dcec003d0.
On small screens with enough horizontal space to show the menu button,
the logo, and the contents of the menu, all three elements were shown on
the same row, which looked broken.
Now the contents of the menu are shown below the menu button.
Note that, to force this, we're making the contents of the menu 100%
wide. That means links would take the 100% of the space, which would
make it easy to click on a link while trying to scroll when using
touchscreens. So we're making the links as wide as their text, which
also has a disadvantage: it's harder to click on narrow links like
"SDG".
We're going to change these styles in order to fix a bug, and the
Foundation styles were getting in the way. Besides, we were overwriting
some rules and so now we're removing 6 properties while we're also
adding 6, so it isn't like the Foundation styles were helping us.
This class was only there in order to use Foundation's styles, but
the amount of styles we were using was equal to the amount of
styles we were overwriting.
This way we follow the convention of one stylesheet per component.
Note that we depend on Foundation's `menu` class for some of the
styles and JavaScript behavior, so we're keeping both the `menu` and
`account-menu` classes.
Since we allow many active budgets at the same time, the
controller should now check the budget given by params.
Before this change the controller was checking the latest
published budget, ignoring the request parameter `budget_id`.
We've noticed the following warning while testing the upgrade to
Ruby 3.0:
warning: File.exists? is deprecated; use File.exist? instead
We're adding a Rubocop rule so we don't call the deprecated method
in the future.
We were passing a hash of options instead of converting them to keyword
parameters, which resulted in warnings on Ruby 2.7:
```
app/components/admin/table_actions_component.html.erb:5: warning: Using
the last argument as keyword parameters is deprecated; maybe ** should
be added to the call
app/components/admin/table_actions_component.html.erb:9: warning: Using
the last argument as keyword parameters is deprecated; maybe ** should
be added to the call
app/components/admin/widget/cards/row_component.html.erb:20: warning:
Using the last argument as keyword parameters is deprecated; maybe **
should be added to the call
app/components/admin/widget/cards/table_component.html.erb:14: warning:
Using the last argument as keyword parameters is deprecated; maybe **
should be added to the call
```
We were using an optional parameter followed by keyword parameters,
which caused a warning with Ruby 2.7:
```
app/components/shared/link_list_component.rb:20: warning: Using the last
argument as keyword parameters is deprecated; maybe ** should be added
to the call
```
I've tried to make `current:` a named parameter as well and then change
all method calls to `link_list`, but was still getting the same warning.
Might have something to do with the fact that we're dealing with arrays
with hashes inside them instead of passing the keyword arguments
directly to the method.
We weren't showing the year when a page was created/updated, and we
were displaying the created date instead of the updated one.
Co-Authored-By: Diego Calvo <diego.calvo@enreda.coop>
We're trying to be consistent; in the past, we had the partials
"shared/admin_login_items", "layouts/notification_item" and
"devise/menu/login_items". Now we're moving all these partials to
components in the `Layout` namespace.
In the management section, `current_user` is the user impersonated by
the manager. We were deciding whether to show the admin menu depending
on the privileges of the current user, but this menu should be shown
according to the privileges of the manager who is impersonating the
user.
We're doing a similar (very subtle) change in the login items. We were
rendering the `login_items` partial passing `current_user: user`.
However, inside this method, we were using `user_signed_in`, which
ignored the `current_user` we were passing. The result was always the
same expect in tests where we manually sign in users, but we're changing
it anyway in order to reduce confusion.
