Skip to content
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

Always show element tabs if the page has fixed elements #2852

Merged
merged 3 commits into from
Apr 30, 2024

Conversation

sascha-karnatz
Copy link
Contributor

@sascha-karnatz sascha-karnatz commented Apr 30, 2024

What is this pull request for?

  • Show always element tabs, if the page has fixed elements
  • moved FixedElements - javascript from assets/javascript to javascript/alchemy_admin
  • migrated to vanilla Javascript

Checklist

  • I have followed Pull Request guidelines
  • I have added a detailed description into each commit message
  • I have added tests to cover this change

@sascha-karnatz sascha-karnatz requested a review from a team as a code owner April 30, 2024 14:17
Copy link

codecov bot commented Apr 30, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 95.86%. Comparing base (4b0437e) to head (da519a0).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2852      +/-   ##
==========================================
- Coverage   95.86%   95.86%   -0.01%     
==========================================
  Files         229      229              
  Lines        6211     6209       -2     
==========================================
- Hits         5954     5952       -2     
  Misses        257      257              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@tvdeyen tvdeyen changed the title Show always element tabs, if the page has fixed elements Always show element tabs if the page has fixed elements Apr 30, 2024
@tvdeyen tvdeyen added this to the 7.2 milestone Apr 30, 2024
Copy link
Member

@tvdeyen tvdeyen left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we can simplify even more

app/views/alchemy/admin/elements/index.html.erb Outdated Show resolved Hide resolved
Remove the step to create the tab group and always show the tab group, if the page has elements, that are fixed. It makes the whole process easier and is in the UI more consistent.
Move createTab and removeTab functions into alchemy_admin - package. These two functions are still available in the Alchemy.FixedElements - namespace until we move all calls out of these js.erb - files. The functions are also migrated to vanilla Javascript.
@sascha-karnatz sascha-karnatz force-pushed the fixed-element-UI-improvements branch from c52c61b to 1e59d50 Compare April 30, 2024 15:46
`Page#available_element_definitions` returns elements still available
for that page. Taking unique and limited elements into account.

`Page#element_definitions` always returns the collections of defined
elements, not matter of they have been used or not.

Due to referencing the same instance variable and in memory object
`available_element_definitions` was mutating the `element_definitions`
object.
@tvdeyen tvdeyen enabled auto-merge April 30, 2024 16:19
@tvdeyen tvdeyen merged commit fad5df2 into AlchemyCMS:main Apr 30, 2024
36 checks passed
@sascha-karnatz sascha-karnatz deleted the fixed-element-UI-improvements branch May 2, 2024 09:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants