BGDIINF_SB-1856 : share - iframe code snippet generator#408
Merged
Conversation
ltshb
reviewed
May 31, 2023
src/store/modules/ui.store.js
Outdated
| * @type Boolean | ||
| */ | ||
| embeddedMode: false, | ||
| embeddedMode: 'true' === new URLSearchParams(window.location.href).get('embed'), |
Contributor
Author
There was a problem hiding this comment.
I was trying to start the app straight into embed mode, without having the UI flicker between the two states. It's not yet as I wanted it to be, so I'll remove that and create a ticket to tackle this detail
Contributor
|
In the shortlink input if you click inside it automatically select the whole input, but on the embed input if you click outside and then back inside nothing is selected. Would be good to have the same behavior for both inputs. |
62c751d to
01c9211
Compare
023104b to
c32d8e9
Compare
Contributor
|
The dropdown button don't uses |
Contributor
|
Personnally I would add a bit more space/padding between the icon and the text here |
ltshb
approved these changes
Jun 14, 2023
Contributor
ltshb
left a comment
There was a problem hiding this comment.
Just the two minor comment above
Now that we support embed=true URL param (with specific UI for this) we can add back the iframe snippet generator portion of the UI with some minor adjustments : - short link used by the iframe snippet generator is a different one from the share section, as it requires to have embed=true as a URL param - some of the structure made a couple month ago was not up to date with the latest changes I also added some e2e Cypress test to cover this iframe snippet modal and its functionalities
- Switch button icon (for embed preview) from "+/-" to caret as on other menu items - rename generateShortLink function as plural, as there is now two short links being generated - better handling of short link backend error, with fallback to non shorten URL in case of error - removing unnecessary complexity with URL param parsing at store init in ui.store.js, a new task will be created to tackle app startup in embed mode so that it never shows the header bar
so that it mimics the behavior of the short link input
Harmonizing margin after icon in the menu tray so that the sections and the "embed" button have the same look Changing the cursor to a pointer when hovering the size selector in the embed popup
Now only embed is added to the url (without the =true part) so the test was failing to detect correctly the backend request to create a short link with the flag Also renaming all actions from the share.store.js file to plural when they are dealing with both the normal short link and the embedded version
c32d8e9 to
4affbcf
Compare
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.
Test link