Added notes to constructor #987
Open
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
app/models/calculator.rb
Outdated
Comment on lines
38
to
39
| validates :english_additional_notes, length: { maximum: 150 } | ||
| validates :ukranian_additional_notes, length: { maximum: 150 } |
There was a problem hiding this comment.
а шо коротке таке? може довше зробити?
| <%= tinymce_assets %> | ||
| <div data-controller="tinymce"> | ||
| <div class="pt-4"> | ||
| <%= f.label :ukranian_additional_notes, "Ukrainian Additional Notes" %> |
| </div> | ||
|
|
||
| <div class="pt-4 pb-4"> | ||
| <%= f.label :english_additional_notes, "English Additional Notes" %> |
loqimean
requested changes
Nov 29, 2024
Comment on lines
1
to
16
| import { Controller } from '@hotwired/stimulus' | ||
|
|
||
| export default class extends Controller { | ||
| static targets = ['input'] | ||
|
|
||
| connect() { | ||
| let config = Object.assign({ target: this.inputTarget }, TinyMCERails.configuration.default ) | ||
| tinymce.init(config) | ||
|
|
||
| } | ||
|
|
||
| disconnect () { | ||
| tinymce.remove() | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
порівняй тут гарно код і винеси конфіги в getter, також юзай 2 пробіли, а не 4 і думаю можна без object assign, просто обʼєкт заюзати
app/models/calculator.rb
Outdated
Comment on lines
38
to
39
| validates :english_additional_notes, length: { maximum: 500 } | ||
| validates :ukranian_additional_notes, length: { maximum: 500 } |
There was a problem hiding this comment.
ну диви, ти молодець, що додала валідацію, але тут не враховані теги, тобто я напишу одну букву, а довжина її буде 7 символів, бо буде огорнута в як мінімум тег p, отож потрібно дещо специфічну валіадацію зробити
| </div> | ||
|
|
||
| <div class="pt-4 pb-4"> | ||
| <%= f.label :english_additional_notes, "English Additional Notes" %> |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## calculators-constructor #987 +/- ##
===========================================================
+ Coverage 89.14% 89.19% +0.05%
===========================================================
Files 68 68
Lines 1059 1064 +5
===========================================================
+ Hits 944 949 +5
Misses 115 115 ☔ View full report in Codecov by Sentry. |
loqimean
requested changes
Dec 4, 2024
app/models/calculator.rb
Outdated
| validates :ukranian_additional_notes, | ||
| length: { | ||
| maximum: 500, | ||
| tokenizer: ->(string) { ActionController::Base.helpers.strip_tags(string).chars } |
There was a problem hiding this comment.
use method to do that and test it
| @@ -0,0 +1,14 @@ | |||
| <%= tinymce_assets %> | |||
There was a problem hiding this comment.
Suggested change
| <%= tinymce_assets %> | |
| <%= tinymce_assets %> |
Comment on lines
4
to
8
| <% if I18n.locale == :uk %> | ||
| <%= raw calculator.ukranian_additional_notes %> | ||
| <% else %> | ||
| <%= raw calculator.english_additional_notes %> | ||
| <% end %> |
There was a problem hiding this comment.
Suggested change
| <% if I18n.locale == :uk %> | |
| <%= raw calculator.ukranian_additional_notes %> | |
| <% else %> | |
| <%= raw calculator.english_additional_notes %> | |
| <% end %> | |
| <%== calculator.notes %> |
then you have:
uk_notes
en_notes
and Translatable concern in a model
config/locales/uk/uk.yml
Outdated
| @@ -361,6 +361,8 @@ uk: | |||
| prohibited_to_update: " перешкоджають оновленню калькулятора" | |||
| prohibited_to_save: " перешкоджають збереженню калькулятора" | |||
| error: "помилки" | |||
| ukranian_additional_notes: "Додаткові нотатки українською" | |||
| english_additional_notes: "Додаткові нотатки англійською" | |||
There was a problem hiding this comment.
у нас же нема основних нотаток)
KseniaGovorun
approved these changes
Dec 26, 2024
loqimean
reviewed
Jan 2, 2025
| <div class="description-block"> | ||
| <%# TODO: ADD text here %> | ||
| <div class="description-section"> | ||
| <%== calculator.notes %> |
There was a problem hiding this comment.
is there any XSS protection?
Co-authored-by: Ivan Marynych <[email protected]>
…projects/ZeroWaste into 950-additional-notes
loqimean
requested changes
Jan 15, 2025
| <div class="description-block"> | ||
| <%# TODO: ADD text here %> | ||
| <div class="description-section"> | ||
| <%= sanitize(calculator.notes, tags: %w[p strong em ul li a div span img], attributes: %w[href target class src alt style]) %> |
loqimean
reviewed
Jan 20, 2025
| <div class="description-block"> | ||
| <%# TODO: ADD text here %> | ||
| <div class="description-section"> | ||
| <%= sanitized_notes(calculator.notes) %> |
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.
Resolves:
Code reviewers
Second Level Review
Summary of issue
The current form for the calculator does not include fields for creating additional notes.
Summary of change
This update introduces two new Ukrainian and English note fields in the calculator form. The changes include creating a YAML file for TinyMCE styling, adding a Stimulus controller to handle TinyMCE initialization, creating a partial for the TinyMCE form, and writing tests for the new fields.
CHECK LIST