Upgrade CONSUL DEMOCRACY to version 2.0.1 #145
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This is one of Foundation's classes that only applies when its parent element is a flex container, which isn't the case here since commit dcec003.
There isn't a `ul` element here, so this rule doesn't apply.
We don't need it since it's only used for flexbox styles, and we already have a `flex-grow` rule for the `h1` element which does the same thing.
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.
The menu didn't look properly on these screens since commit dcec003. 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 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.
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 dcec003, and we were overriding all the Foundation styles in the admin area with the exception of the padding, which we no longer need.
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.
We weren't using the `admin-top-bar` class since commit e6c1cf7, 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 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.
So we're consistent with the rest of the code in the header, which renders components and not partials.
That way it's possible to override the style without changing the HTML, which is the hardest code to customize and maintain.
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 701378d.
Instead of adding all the styles of a callout and then overwriting half of them, we can simply add the half we need.
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.
Not sure why these properties were added, but they no longer seem to be necessary.
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.
This way it'll be easier to change the code.
Now that all the code related to this model is in the same place, changing it will be easier.
This way we can simplify the code dealing with the translatable association.
I was finding this part hard to follow, but after changing the name of the condition suddenly I understood what was going on.
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.
Fix menu on "wide" small screens
…ns_crash Fix crash translating an already translated text
This way we reduce the number of system tests or, in some cases, requests during system tests, making the tests faster. We're still testing the interaction with the menu when users have the right permissions.
In most sections, we had two specs testing what happens after accessing one of the privileged areas. We're grouping the expectations and so we've only got one test per area, making these tests faster.
Since the change on commit cbbe188 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.
Always show poll officer menu to officers
We've experienced some caching issues with this code for years. We've fixed some of them in commits 9979b53 and 3645c33, 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.
We're going to add more cases and the test for the link to the evaluate would become too big, so we're splitting it.
…enium_webdriver_to_4.11 Bump selenium-webdriver from 4.0.0 to 4.11.0
In the `i18n_translation` initializer, we're overwriting the `t` helper so calling it uses custom translations if they're available. However, ViewComponent doesn't use the `t` helper but implements its own `t` method. So, when calling the `t` method in a component, we weren't using our implementation of the `t` helper, and so we weren't loading custom translations. Using the `t` helper in components solves the issue. There was a test where we were directly testing a method in a component, and that method uses the `t` helper. This caused an error when running the test: ViewComponent::Base::ViewContextCalledBeforeRenderError: `#helpers` can't be used during initialization, as it depends on the view context that only exists once a ViewComponent is passed to the Rails render pipeline. Using `render_inline` in the test and testing the generated HTML, as recommended in the ViewComponent documentation, solves the issue.
We added the `user_id` rule in commit edaf420. To be honest, I'm not sure what we meant, since I haven't found URLs containing the user id. So we're treating it as if it was a typo and we wanted to do the same thing we did with other parameters.
Not sure exactly since when, but we've started to get rubocop failures when using the pattern: respond_to do |format| format.js end We've been using this pattern for ages, so maybe a recent version of Rubocop introduced a change that made it report it. In any case, this is easier to read than respond_to(&:js), so we're allowing it.
Fix syntax in robots.txt
…ranslations_in_components Use custom translations in components
Fix Rubocop convention offenses
…wdin Update translations from Crowdin
…2.0.1 Release version 2.0.1
microweb10
force-pushed
the
release_2.0.1
branch
from
23b3df2
to
769b9a1
Compare
Move custom specs to a custom folder
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Objectives
Upgrade CONSUL DEMOCRACY to version 2.0.1 🎉
Notes
Release notes 2.0.0
Release notes 2.0.1