Apply flash styles to Announcements#11708
Merged
Conversation
…tually an announcement
* Remove `.flex-col`, as when there are links it breaks. For instance, in the decidim-elections Bulletin Board not configured message in the Admin panel. * Replace `.font-bold` with `.font-semibold`, reducing the boldness * Add underline to links, so users can know that they're clickable
… with AnnouncementCell There are some cases where we can't use the Cell, as there are links and there's a sanititzation process. On these cases we add a comment and style them manually.
This rule was overriding all of the SVG, so we relax it a bit.
a8eb40b to
1c254bf
Compare
1c254bf to
8ec8f85
Compare
Until now the Javascript `hidden` function worked without issues, but now that we're changing classes of this element, as the `flash` class has already a display, it doesn't get hidden. The culprit is: > Web browsers may implement the hidden state using display: none, in which case the > element will not participate in page layout. This also means that changing the value > of the CSS display property on an element in the hidden state will override the state. > For instance, elements styled display: block will be displayed despite the hidden > attribute's presence. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden#the_hidden_state So, with a god old `display: none` we can fix this problem.
While working in the `./decidim-core/spec/system/account_spec.rb`, I've detected that we have not enabled the headless option, meaning that it is opening pages in a visibly browser. This is a bit weird because it's the only driver that's doing that. This commit disables that behavior so its more consistent.
8ec8f85 to
4168add
Compare
I left a comment on why I'm reverting this. Basically, the offline navigation feature doesn't work with the headless browser.
alecslupu
suggested changes
Oct 4, 2023
Contributor
alecslupu
left a comment
There was a problem hiding this comment.
I would change a bit the ones that i have commended on ...
Suggested by code review
It was implemented in two places and it didn't read well, so we're removing this altogether.
entantoencuanto
added a commit
that referenced
this pull request
Oct 6, 2023
* feature/renaming-redesign: Use default current participatory space scope as root on scopes_select_tag called from bulk actions restore the imports thing (compilation fails for initiatives) change testing color: dequelabs/axe-core#3513 (comment) Fix admin redesign module (#11648) Apply flash styles to Announcements (#11708) Refactor oneliners of redesigned_a11y.js (#11713) Redesign: fix votings admin module issues (#11704) Add meta robots noindex to search and profile (#10120) Refactor dropdown scroll to menu (#11710) Remove unused partial Remove REDESIGN_PENDING obsolete comand Redesign: fix assembly members sidebar menu (#11699) Redesign: fix conferences admin module issues (#11703) Redesign: fix assemblies admin module issues (#11702)
andreslucena
added a commit
that referenced
this pull request
Oct 16, 2023
* Apply HTML from flashes to announcements * Inline CSS classes to HTML callout to fix usage while the flash is actually an announcement * Migrate hardcoded callouts to AnnouncementCell * Fix flash styles * Remove `.flex-col`, as when there are links it breaks. For instance, in the decidim-elections Bulletin Board not configured message in the Admin panel. * Replace `.font-bold` with `.font-semibold`, reducing the boldness * Add underline to links, so users can know that they're clickable * Make the flash alerts manually in the cases where it can't be created with AnnouncementCell There are some cases where we can't use the Cell, as there are links and there's a sanititzation process. On these cases we add a comment and style them manually. * Fix bug with size of SVG icons in Admin This rule was overriding all of the SVG, so we relax it a bit. * Fix columns on flash when there's a title in the AnnouncementCell * Fix `have_admin_callout` matcher with multiple flashes in the same page * Fix specs * Add full flash messages on 'has questionnaire' shared spec * Improve `css_class` method when there isn't any `callout_class` argument * Normalize i18n files with i18n-tasks * Fix hidding when canceling email change in account#show Until now the Javascript `hidden` function worked without issues, but now that we're changing classes of this element, as the `flash` class has already a display, it doesn't get hidden. The culprit is: > Web browsers may implement the hidden state using display: none, in which case the > element will not participate in page layout. This also means that changing the value > of the CSS display property on an element in the hidden state will override the state. > For instance, elements styled display: block will be displayed despite the hidden > attribute's presence. https://developer.mozilla.org/en-US/docs/Web/HTML/Global_attributes/hidden#the_hidden_state So, with a god old `display: none` we can fix this problem. * Make the `pwa_chrome` Capybara driver not open pages While working in the `./decidim-core/spec/system/account_spec.rb`, I've detected that we have not enabled the headless option, meaning that it is opening pages in a visibly browser. This is a bit weird because it's the only driver that's doing that. This commit disables that behavior so its more consistent. * Revert "Make the pwa_chrome Capybara driver not open pages" I left a comment on why I'm reverting this. Basically, the offline navigation feature doesn't work with the headless browser. * Change CSS class for callout Suggested by code review * Drop truncate support from AnnouncementCell It was implemented in two places and it didn't read well, so we're removing this altogether.
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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.
🎩 What? Why?
As I mentioned when I introduced the new flashes in the admin panel (#11665):
So, this is the PR missing, with the change of the announcements to use the new design.
Apart from that, I tuned a bit the CSS of the flashes, as I found them too bold.
Its worth noticing two particular cases where we used links in the texts, so on those cases I preferred to hard-code the styling, as the alternative was relaxing the security in the AnnouncementCell clean_body method and I didn't like that
📌 Related Issues
Testing
📷 Screenshots