-
-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Create and Edit Product Property with UI components issue #5857 #5892
base: main
Are you sure you want to change the base?
Create and Edit Product Property with UI components issue #5857 #5892
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Astr0surf3r You have not followed our pull request guidelines:
- The commit messages lack any explanation.
- Some commits add unnecessary code that gets removed by later commits.
- Linting changes shouldn't be in separate commits and your linting commit also removes unneeded code.
I'd appreciate a rebase of this PR to address these issues.
@MadelineCollier Can you take a pass over this from an implementation perspective and make sure it's consistent with how similar features are built? You have better knowledge of how admin features should be built.
def form_id | ||
dom_id(@property, "#{stimulus_id}_edit_property_form") | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This file is mostly misindented.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jarednorman thank you for your message I have just already made rebase in this PR and follow the solidus guideline let me know if need a cleaner PR I can upload 1 single commit with all changes ... let me know please
c399ad5
to
042abb4
Compare
042abb4
to
f52b0cf
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #5892 +/- ##
==========================================
+ Coverage 89.58% 89.61% +0.02%
==========================================
Files 783 785 +2
Lines 17989 18049 +60
==========================================
+ Hits 16116 16174 +58
- Misses 1873 1875 +2 ☔ View full report in Codecov by Sentry. |
Summary
fix the issue #5857
Now to add a product property appear a modal
And to edit a Property appear a modal as well
[x] I have written a thorough PR description.
[x] I have kept my commits small and atomic.
[x] I have used clear, explanatory commit messages.
✅ I have added automated tests to cover my changes.
📸 I have attached screenshots to demo visual changes.