feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '
Skip to content

feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix"); } } catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms"); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links"); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + '
Skip to content

feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default"); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers"); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + '
Skip to content

feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode"); } } catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); } })(); })();
Skip to content

feat(Dropdowns): Clockface 4 dropdown updates - #876

Merged
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates
Oct 19, 2022
Merged

feat(Dropdowns): Clockface 4 dropdown updates#876
ChitlangeSahas merged 5 commits into
clockface-4-masterfrom
clockface-4-dropdown_updates

Conversation

@ChitlangeSahas

Copy link
Copy Markdown
Contributor

Closes#838

Couple screenshots:

SelectDropdown:

Screen Shot 2022-10-17 at 9 22 16 PM

TypeAheadDropdown:

Screen Shot 2022-10-17 at 9 27 58 PM

CreateableTypeAheadDropdown:

Screen Shot 2022-10-17 at 9 28 43 PM

MultiSelect:

Screen Shot 2022-10-17 at 9 29 16 PM

  • Updated documentation to reflect changes
  • Added entry to top of Changelog with link to PR (not issue)
  • Tests pass
  • Peer reviewed and approved
  • Signed CLA (if not already signed)

@ChitlangeSahasChitlangeSahas changed the title Clockface 4 dropdown updatesfeat(Dropdowns): Clockface 4 dropdown updatesOct 18, 2022
@ChitlangeSahas
ChitlangeSahas changed the base branch from master to clockface-4-masterOctober 18, 2022 04:30
Comment on lines +56 to +57
margin-right: 2px;
margin-left: 1px;

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

These two stick out, but they're just to please the absolute positioned, Scrollbar library we're using

@hoorayimhelpinghoorayimhelpingOct 18, 2022

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can this be solved using position and left and right rather than with using margins? Semantically, there's a big difference between position and margin, and we should use positioning for laying things out, and margins for when we need space around an element. I'm not super familiar with how this is used, though

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Got you! I'll try that!

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 8 19 39 AM

It still cuts off the right end of it, I think this is what is happening:

IMG_D4759C8952C6-1

So, when I add the margin, the border falls under the available space.

Unfortunately, I am not able to change the width of the inner wrapper, since it's controlled by the react-custom-scrollbar library.

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

This was always a problem, but we never noticed because we didn't use borders as an indicator for being selected. Now that we use the border, we see that the 1px of the right is cut off.

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

is it causing problems to have the border? julia and i also discussed maybe not needing a background color at all on selected items. we included it because that's what we have now—adjusted to be lower contrast so it's less distracting—but i think the selected state for the radio buttons and checkboxes has enough contrast that we might not need the background color+border as well. let me know what you think. @ChitlangeSahas@hoorayimhelping@juliajames12

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Currently I implemented it in the way figma looks, with no issues. But if we decide to remove the border+background, let me know I can make a future ticket and address that! 👍

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

talked to sahas—it's a big of a bigger project to reevaluate these, so we'll keep it as-is for now and tweak it later if we find we need to once we test it out

Comment on lines +225 to +230
@include buttonColorModifier(
$cf-turquoise,
$cf-turquoise,
$cf-grey-1,
$cf-grey-1
);

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Just ran prettier, hence the diff.

@ChitlangeSahas
ChitlangeSahas marked this pull request as ready for review October 18, 2022 04:32
@ChitlangeSahas
ChitlangeSahas requested a review from a teamOctober 18, 2022 04:32
@include dropdownItemStyles();
font-weight: $cf-font-weight--medium;
text-transform: uppercase;
color: #88889b;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Since this is just a divider - does text-transform: uppercase have any effect?

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

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

Screen Shot 2022-10-18 at 9 17 47 AM

The dividers have text, optionally 👍

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Gotcha - thanks!

@ChitlangeSahas
ChitlangeSahas merged commit 0c4cb93 into clockface-4-masterOct 19, 2022
@ChitlangeSahas
ChitlangeSahas deleted the clockface-4-dropdown_updates branch October 19, 2022 16:39
Sign up for freeto 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.

Clockface 4.0: Dropdowns

4 participants

@ChitlangeSahas@hoorayimhelping@taramk@wdoconnell