-
Notifications
You must be signed in to change notification settings - Fork 177
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
ARC-1700 Skeleton loading for create branch #1652
Conversation
.skeleton, | ||
.skeleton > label { | ||
/* Override AUI styling */ | ||
color: transparent !important; |
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.
I definitely don't like using this but unfortunately the styling of AUI components really get in the way here and override this style without it.
views/github-create-branch.hbs
Outdated
<div class="gitHubCreateBranch__subHeader"> | ||
Creating a branch for <b>{{issue.key}}</b> | ||
</div> | ||
<div class="gitHubCreateBranch__subHeader skeleton skeleton-text">Creating a branch for <b>{{issue.key}}</b></div> |
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.
For most elements, simply adding skeleton and skeleton-element will make it possible implement this loading state.
views/github-create-branch.hbs
Outdated
{{/each}} | ||
</select> | ||
<aui-label for="server" class="skeleton skeleton-text">GitHub Enterprise Server instance</aui-label> | ||
<div class="skeleton skeleton-input"></div> |
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.
For input elements, we need to handle it a little differently because the AUI styling gets in the way and I didn't want to end up with !important all over the place. Easier solution here is to simply render a separate skeleton element temporarily in place of the real element.
@@ -0,0 +1,9 @@ | |||
$(document).ready(function() { | |||
|
|||
setTimeout(function() { |
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.
I had originally planned to use a handlebars conditional to handle rendering the skeleton element vs the real one but turns out that isn't possible. When the page loads, we already have the data e.g. repos. The issue with the delayed rendering appears to be due to how we're looping over that data within the input fields. So, because the data is already there when the page loads, we're simply displaying the skellie stuffs for half a second before moving it and displaying the real element.
Curious on thoughts are the half second wait time. Seems to work for this page but may differ with more data?
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.
ahhhh thats inconvenient. So is it the select2 elements that are slow?
They might have an event when they are ready?
Since the repo data gets passed with the template(think theres about a 20 repos max) we should see a pretty consistent load..... we could try force 100+ or so and see how it reacts.
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.
Nah it's not just the select element. Repository is janky too and that's just a div but I think it's the looping that's slowing the rendering down.
{{#each repos}}
<div class="hidden default-repos">{{node.full_name}}</div>
{{/each}}
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.
Looks like the max we can pass is 100. Just tested it out and still looks good.
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.
👍
4e52303
to
0508bdf
Compare
What's in this PR?
Skeleton loading for CB form page.
Screen.Recording.2022-10-09.at.8.54.02.pm.mov
Why
Currently, when the form for this page loads it looks like this:
We debated using the loading spinner which is used elsewhere in the app vs skeleton loading which is now used across Jira. We decided it would be better to align with the latter as customers will be navigating to this screen from Jira and it will provide a more seamless experience. Unfortunately, AUI doesn't provide a skeleton component (found out they were only built for Atlaskit) so we've had to handle this ourselves.
Added feature flags
CB
Affected issues
ARC-1700
Whats Next?
Update other parts of the app to use skeleton loading so that our entire app aligns with what's now used across Atlassian products.