Implement detail page skeleton by ddevault · Pull Request #80 · linode/manager

ddevault · 2016-06-07T19:54:39Z

Closes #30. I still have to write all of the tests for this.

eatonphil · 2016-06-07T19:56:20Z

 .li-dropdown {
-    color: #999;
+    color: #555;
+    position: relative;


What was the reason for this change?

@na3d pointed out that the width of the dropdown's contents don't match up with the width of the dropdown. This fixes that.

(with the help of other changes)

Ah. I wouldn't worry about this too much. It's too complex and arbitrary. I do not expect for much of the dropdown code to remain for long.

Cool. I was thinking that an overhaul of the dropdowns was probably necessary. Want to backlog it?

Update a few styles

ddevault · 2016-06-08T18:11:36Z

This is ready for review+merge once build passes.

coveralls · 2016-06-08T18:14:52Z

Coverage increased (+6.2%) to 75.887% when pulling 3e52a11 on feature/detail-page-skeleton into 208b744 on master.

na3d · 2016-06-08T18:19:12Z

👍

eatonphil · 2016-06-08T18:20:05Z

+    this.renderHeader = this.renderHeader.bind(this);
+    this.renderEditUI = this.renderEditUI.bind(this);
+    this.renderLabel = this.renderLabel.bind(this);
+    this.handleLabelKeyUp = this.handleLabelKeyUp.bind(this);


It's a little late, but I am not a big fan of this.

A fan of what?

Reassigning the same variable with this bound.

This is a common React pattern. We don't really have a choice. They need to be bound to this.

they don't need to be bound to a variable 2) they don't need to be bound to the same variable.

They will only cause errors in certain contexts - generally if someone just doesn't know what they're doing.

That's not true. If we have correct test coverage (which we should verify on each PR) they will always verify every code path, including the ones that would use this and then fail.

There's still nothing to stop someone from not doing this.callback = this.callback.bind(this) and calling <Thing onClick={this.callback.bind(this)} /> instead and so forth.

We shouldn't defy established convention because it gives you the willies. Your solution also introduces more boilerplate and transfers responsibility for binding to the caller, where I expect mistakes are more likely to happen. 👎

I'm saying your suggestions are less valid.

eatonphil · 2016-06-08T19:12:52Z

ready for merge?

ddevault · 2016-06-08T19:13:10Z

Yes.

ddevault · 2016-06-08T19:14:08Z

Thanks.

upcoming: [DI-21119] - token call fix for differently ordered same resource ids..

…1109] Improve Vitest setup w/ shards" Merge in FEE/cloud-manager from chore/cicd-tests to develop Squashed commit of the following: commit 8f53c371bb83fa794a3dd948f954b0ecd233f4ea Author: Conal Ryan <corya@akamai.coms> Date: Fri Apr 17 12:45:56 2026 -0400 Revert "Pull request linode#73: chore: [UIE-11109] Improve Vitest setup w/ shards" This reverts commit 4cb03e5.

…1109] Improve Vitest setup w/ shards" Merge in FEE/cloud-manager from chore/cicd-tests to develop Squashed commit of the following: commit 8f53c371bb83fa794a3dd948f954b0ecd233f4ea Author: Conal Ryan <corya@akamai.coms> Date: Fri Apr 17 12:45:56 2026 -0400 Revert "Pull request linode#73: chore: [UIE-11109] Improve Vitest setup w/ shards" This reverts commit 6683d46.

…e Vitest setup w/ shards" Merge in FEE/cloud-manager from chore/cicd-tests to develop Squashed commit of the following: commit 8f53c371bb83fa794a3dd948f954b0ecd233f4ea Author: Conal Ryan <corya@akamai.coms> Date: Fri Apr 17 12:45:56 2026 -0400 Revert "Pull request #73: chore: [UIE-11109] Improve Vitest setup w/ shards" This reverts commit ae3f7f6.

Drew DeVault added 8 commits June 7, 2016 10:05

Add reducer for detail page

bae7917

Initial pass on the detail page skeleton

4760ce6

Wire up power buttons, tweak styles

16be9b9

Extend reducers/actions & add tests for edit mode

ccf4056

Implement editing Linode label/group

682cada

Tweak dropdown styles

2991ff2

Disable save/cancel while request is pending

ebce1be

Set up stubs for detail page tests

fbac333

eatonphil reviewed Jun 7, 2016
View reviewed changes

Drew DeVault and others added 8 commits June 8, 2016 09:51

Move button styles to top level stylesheet

53e84af

Add tab hover

15597d2

Add button styles

c9bf5ec

Merge pull request #84 from eatonphil/feature/detail-page-skeleton

00ebe16

Update a few styles

Darken the header background color

401421f

Add tests for commitChanges action

f33cd34

Implement detail page tests, except for editing UI

4143e52

Flesh out detail page tests

3e52a11

ddevault changed the title ~~[DO NOT MERGE] Implement detail page skeleton~~ Implement detail page skeleton Jun 8, 2016

eatonphil reviewed Jun 8, 2016
View reviewed changes

eatonphil merged commit 02022c0 into master Jun 8, 2016

ddevault deleted the feature/detail-page-skeleton branch June 8, 2016 19:14

nikhagra-akamai pushed a commit to nikhagra-akamai/manager that referenced this pull request Nov 22, 2024

Merge pull request linode#80 from ACLPManager/feature/jweTokenLimit

5b05e9a

upcoming: [DI-21119] - token call fix for differently ordered same resource ids..

Uh oh!

Conversation

ddevault commented Jun 7, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ddevault commented Jun 8, 2016

Uh oh!

coveralls commented Jun 8, 2016

Uh oh!

na3d commented Jun 8, 2016

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eatonphil Jun 8, 2016 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eatonphil commented Jun 8, 2016

Uh oh!

ddevault commented Jun 8, 2016

Uh oh!

ddevault commented Jun 8, 2016

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

eatonphil Jun 8, 2016 •

edited

Loading