Skip to content

Conversation

@LSantha
Copy link
Collaborator

@LSantha LSantha commented Aug 5, 2021

Added version 2 for the following components to make them usable in the page editor: accountedtails, addressbook, minicart, resetpassword, searchbar, miniaccount.

Related Issue

CIF-1502

How Has This Been Tested?

Manually.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to change)

Checklist:

  • I have signed the Adobe Open Source CLA.
  • My change requires a change to the documentation.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes and the overall coverage did not decrease.
  • All unit tests pass on CircleCi.
  • I ran all tests locally and they pass.

 * added version 2 for the following components to make them usable in the page editor: accountedtails, addressbook, minicart, resetpassword, searchbar, miniaccount
@LSantha LSantha marked this pull request as draft August 5, 2021 16:29
@codecov
Copy link

codecov bot commented Aug 5, 2021

Codecov Report

Merging #650 (6063000) into master (93bd569) will not change coverage.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master     #650   +/-   ##
=========================================
  Coverage     88.41%   88.41%           
  Complexity     1593     1593           
=========================================
  Files           280      280           
  Lines          6975     6975           
  Branches       1045     1045           
=========================================
  Hits           6167     6167           
  Misses          610      610           
  Partials        198      198           
Flag Coverage Δ
integration 59.49% <ø> (ø)
jest 85.61% <ø> (ø)
karma 89.05% <ø> (ø)
unittests 88.97% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 93bd569...6063000. Read the comment docs.

@LSantha LSantha added the feature New feature or request label Aug 5, 2021
@LSantha LSantha marked this pull request as ready for review August 10, 2021 10:46
@herzog31
Copy link
Member

@LSantha What do you think about introducing a generic "Frontend Placeholder" component instead of having 5 individual placeholder components? The component could have a dialog with a select field to select between predefined placeholders (accountedtails, addressbook, minicart, resetpassword, miniaccount) and a free textfield to use a custom CSS class in case customers want to add additional frontend components.

@LSantha
Copy link
Collaborator Author

LSantha commented Aug 10, 2021

I think from an authoring standpoint and also for future evolution the current approach is better.
With a proper placeholder component the related react component has a proper representation and identity in the editor and in the authors mind. Instead of removing the placeholders, we could think for instance about what could be made configurable in each of those react components in the future and add options in the edit dialog of the component, but other usages could be considered too.

@dplaton dplaton merged commit 00da37a into master Aug 17, 2021
@dplaton dplaton deleted the CIF-1502 branch August 17, 2021 11:05
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

feature New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants