-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
New design of the forms for creating debates and proposals #4225
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
b7b3e5e
to
11d2ed3
Compare
5493486
to
b273fb5
Compare
3a23f55
to
975d882
Compare
We're going to rewrite most of the code, so we might as well treat it like we treat new files.
9f1c763
to
c23a8b1
Compare
23e4ec6
to
94e3c4f
Compare
taitus
approved these changes
Jul 13, 2021
Just like it's done in the investment form.
So it's consistent with the proposal-edit class we use in the edit action.
So they follow the same convention used in proposals. Note the styles are for elements which appear in the "new" view but not in the "edit" view, so we only have to include them in one place.
We don't need any row classes anymore because the <body> already has a maximum width. As for columns, we only have one column in this form, so we don't need them either. Besides, the form's parent element already has a padding.
Now the padding is only applied in two places (administration forms) so we can apply it just there instead of applying it everywhere and then removing it in most places. We're using the `column` class here because it's what's used in the rest of the fields of these forms and because we haven't defined (yet) general margin/padding rules for the administration views.
It looks like Internet Explorer wasn't applying the padding to the <main> element because it considered it an inline element.
We had an additional `<div>` just to add a background color, when we can do it by applying the background color to the whole `<main>` element and then the body background color to the optional fields. However, I've decided not to do so. The main purpose of changing the background color is to highlight the required fields. The benefits of changing the background color of the header as well are unclear. When in doubt, we're using the solution which requires less code.
In commit 49b4061 we added an extra `<span>` element just so we could add an icon to the right while maintaining both the title and subtitle on the left. We can do the same thing without the extra `<span>` element, absolutely positioning the element and leaving enough padding.
That way we'll be able to simplify some of the code.
The organization name is helpful to screen reader users when they've got several tabs/windows open with different sites.
So we don't add the same lines to pretty much every stylesheet we create. Eventually we'll remove this code and add a padding to every <main> element, or (even better) to the <body> element itself.
Since we're going to reuse this pattern in other forms, we shouldn't rely on the header having just one element. There could be a subtitle. So we're changing the CSS to be less dependent on a very specific HTML structure. Regarding the subtitle, the original idea was to have both an <h1> and an <h2> element inside the header. However, the W3C advices against it [1]: > h1–h6 elements must not be used to markup subheadings, subtitles, > alternative titles and taglines unless intended to be the heading for > a new section or subsection. So we ended up including the subtitle inside he <h1>. We could also add it in a separate <p> tag. However, in this case I think it's better to include it in the <h1> (and in the <title> tag) because it helps to uniquely identify the current page from other pages. Due to some rounding issues in Firefox, we're manually moving the polygon 6px so there isn't a blank space between it and the icon on the right. And due to rounding issues in Chrome, we're adding one extra pixel to the bottom of the polygon defining the clip-path. [1] https://www.w3.org/TR/html52/common-idioms-without-dedicated-elements.html#common-idioms-without-dedicated-elements
The same way we don't show recommendations when editing a proposal or a debate.
The same way we did with debates and proposals, we move recommendations before the form.
Now that we display them in one column, the lines were too long for a small font size.
We've deprecated the "icons" font since we started using Font Awesome two years ago and using it caused some screen readers to announce an "l" before the content of every list item.
taitus
approved these changes
Jul 13, 2021
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.
References
Objectives
According to the new designs introduced in #4198 for participatory budgets, this PR includes a new design of the forms for creating debates and proposals.
Visual Changes
New debate form
New proposal form